Skip to content

Commit 9ae6940

Browse files
committed
fix mixture of sync/async sockets in IOPubThread
all sockets are explicitly sync until/except we are in the coroutines that will await them - consistent behavior of send for child pipe and main process sockets - avoids unsafe assumption that send is greedy on async sockets - avoids potential issues creating async objects in one thread, then using them in another in a different event loop - always creates/uses the right types, regardless of input socket - address some typing lint
1 parent 314cc49 commit 9ae6940

3 files changed

Lines changed: 48 additions & 33 deletions

File tree

ipykernel/inprocess/ipkernel.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import logging
77
import sys
88
from contextlib import contextmanager
9+
from typing import cast
910

1011
from anyio import TASK_STATUS_IGNORED
1112
from anyio.abc import TaskStatus
@@ -146,7 +147,8 @@ def callback(msg):
146147
assert frontend is not None
147148
frontend.iopub_channel.call_handlers(msg)
148149

149-
self.iopub_thread.socket.on_recv = callback
150+
iopub_socket = cast(DummySocket, self.iopub_thread.socket)
151+
iopub_socket.on_recv = callback
150152

151153
# ------ Trait initializers -----------------------------------------------
152154

ipykernel/inprocess/socket.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,6 @@ async def poll(self, timeout=0):
6363
assert timeout == 0
6464
statistics = self.in_receive_stream.statistics()
6565
return statistics.current_buffer_used != 0
66+
67+
def close(self):
68+
pass

ipykernel/iostream.py

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
# Copyright (c) IPython Development Team.
44
# Distributed under the terms of the Modified BSD License.
55

6+
from __future__ import annotations
7+
68
import atexit
79
import contextvars
810
import io
@@ -15,7 +17,7 @@
1517
from collections import defaultdict, deque
1618
from io import StringIO, TextIOBase
1719
from threading import Event, Thread, local
18-
from typing import Any, Callable, Deque, Dict, Optional
20+
from typing import Any, Callable
1921

2022
import zmq
2123
from anyio import create_task_group, run, sleep, to_thread
@@ -25,8 +27,8 @@
2527
# Globals
2628
# -----------------------------------------------------------------------------
2729

28-
MASTER = 0
29-
CHILD = 1
30+
_PARENT = 0
31+
_CHILD = 1
3032

3133
PIPE_BUFFER_SIZE = 1000
3234

@@ -87,15 +89,19 @@ def __init__(self, socket, pipe=False):
8789
Whether this process should listen for IOPub messages
8890
piped from subprocesses.
8991
"""
90-
self.socket = socket
92+
# ensure all of our sockets as sync zmq.Sockets
93+
# don't create async wrappers until we are within the appropriate coroutines
94+
self.socket: zmq.Socket[bytes] | None = zmq.Socket(socket)
95+
self._sync_context: zmq.Context[zmq.Socket[bytes]] = zmq.Context(socket.context)
96+
9197
self.background_socket = BackgroundSocket(self)
92-
self._master_pid = os.getpid()
98+
self._main_pid = os.getpid()
9399
self._pipe_flag = pipe
94100
if pipe:
95101
self._setup_pipe_in()
96102
self._local = threading.local()
97-
self._events: Deque[Callable[..., Any]] = deque()
98-
self._event_pipes: Dict[threading.Thread, Any] = {}
103+
self._events: deque[Callable[..., Any]] = deque()
104+
self._event_pipes: dict[threading.Thread, Any] = {}
99105
self._event_pipe_gc_lock: threading.Lock = threading.Lock()
100106
self._event_pipe_gc_seconds: float = 10
101107
self._setup_event_pipe()
@@ -106,7 +112,7 @@ def __init__(self, socket, pipe=False):
106112

107113
def _setup_event_pipe(self):
108114
"""Create the PULL socket listening for events that should fire in this thread."""
109-
ctx = self.socket.context
115+
ctx = self._sync_context
110116
self._pipe_in0 = ctx.socket(zmq.PULL)
111117
self._pipe_in0.linger = 0
112118

@@ -141,8 +147,7 @@ def _event_pipe(self):
141147
event_pipe = self._local.event_pipe
142148
except AttributeError:
143149
# new thread, new event pipe
144-
ctx = zmq.Context(self.socket.context)
145-
event_pipe = ctx.socket(zmq.PUSH)
150+
event_pipe = self._sync_context.socket(zmq.PUSH)
146151
event_pipe.linger = 0
147152
event_pipe.connect(self._event_interface)
148153
self._local.event_pipe = event_pipe
@@ -161,9 +166,11 @@ async def _handle_event(self):
161166
Whenever *an* event arrives on the event stream,
162167
*all* waiting events are processed in order.
163168
"""
169+
# create async wrapper within coroutine
170+
pipe_in = zmq.asyncio.Socket.shadow(self._pipe_in0)
164171
try:
165172
while True:
166-
await self._pipe_in0.recv()
173+
await pipe_in.recv()
167174
# freeze event count so new writes don't extend the queue
168175
# while we are processing
169176
n_events = len(self._events)
@@ -177,7 +184,7 @@ async def _handle_event(self):
177184

178185
def _setup_pipe_in(self):
179186
"""setup listening pipe for IOPub from forked subprocesses"""
180-
ctx = self.socket.context
187+
ctx = self._sync_context
181188

182189
# use UUID to authenticate pipe messages
183190
self._pipe_uuid = os.urandom(16)
@@ -199,6 +206,8 @@ def _setup_pipe_in(self):
199206

200207
async def _handle_pipe_msgs(self):
201208
"""handle pipe messages from a subprocess"""
209+
# create async wrapper within coroutine
210+
self._async_pipe_in1 = zmq.asyncio.Socket(self._pipe_in1)
202211
try:
203212
while True:
204213
await self._handle_pipe_msg()
@@ -209,8 +218,8 @@ async def _handle_pipe_msgs(self):
209218

210219
async def _handle_pipe_msg(self, msg=None):
211220
"""handle a pipe message from a subprocess"""
212-
msg = msg or await self._pipe_in1.recv_multipart()
213-
if not self._pipe_flag or not self._is_master_process():
221+
msg = msg or await self._async_pipe_in1.recv_multipart()
222+
if not self._pipe_flag or not self._is_main_process():
214223
return
215224
if msg[0] != self._pipe_uuid:
216225
print("Bad pipe message: %s", msg, file=sys.__stderr__)
@@ -225,14 +234,14 @@ def _setup_pipe_out(self):
225234
pipe_out.connect("tcp://127.0.0.1:%i" % self._pipe_port)
226235
return ctx, pipe_out
227236

228-
def _is_master_process(self):
229-
return os.getpid() == self._master_pid
237+
def _is_main_process(self):
238+
return os.getpid() == self._main_pid
230239

231240
def _check_mp_mode(self):
232241
"""check for forks, and switch to zmq pipeline if necessary"""
233-
if not self._pipe_flag or self._is_master_process():
234-
return MASTER
235-
return CHILD
242+
if not self._pipe_flag or self._is_main_process():
243+
return _PARENT
244+
return _CHILD
236245

237246
def start(self):
238247
"""Start the IOPub thread"""
@@ -265,7 +274,8 @@ def close(self):
265274
self._pipe_in0.close()
266275
if self._pipe_flag:
267276
self._pipe_in1.close()
268-
self.socket.close()
277+
if self.socket is not None:
278+
self.socket.close()
269279
self.socket = None
270280

271281
@property
@@ -301,12 +311,12 @@ def _really_send(self, msg, *args, **kwargs):
301311
return
302312

303313
mp_mode = self._check_mp_mode()
304-
305-
if mp_mode != CHILD:
306-
# we are master, do a regular send
314+
if mp_mode != _CHILD:
315+
# we are the main parent process, do a regular send
316+
assert self.socket is not None
307317
self.socket.send_multipart(msg, *args, **kwargs)
308318
else:
309-
# we are a child, pipe to master
319+
# we are a child, pipe to parent process
310320
# new context/socket for every pipe-out
311321
# since forks don't teardown politely, use ctx.term to ensure send has completed
312322
ctx, pipe_out = self._setup_pipe_out()
@@ -379,7 +389,7 @@ class OutStream(TextIOBase):
379389
flush_interval = 0.2
380390
topic = None
381391
encoding = "UTF-8"
382-
_exc: Optional[Any] = None
392+
_exc: Any = None
383393

384394
def fileno(self):
385395
"""
@@ -470,14 +480,14 @@ def __init__(
470480
self.pub_thread = pub_thread
471481
self.name = name
472482
self.topic = b"stream." + name.encode()
473-
self._parent_header: contextvars.ContextVar[Dict[str, Any]] = contextvars.ContextVar(
483+
self._parent_header: contextvars.ContextVar[dict[str, Any]] = contextvars.ContextVar(
474484
"parent_header"
475485
)
476486
self._parent_header.set({})
477487
self._thread_to_parent = {}
478488
self._thread_to_parent_header = {}
479489
self._parent_header_global = {}
480-
self._master_pid = os.getpid()
490+
self._main_pid = os.getpid()
481491
self._flush_pending = False
482492
self._subprocess_flush_pending = False
483493
self._buffer_lock = threading.RLock()
@@ -569,8 +579,8 @@ def _setup_stream_redirects(self, name):
569579
self.watch_fd_thread.daemon = True
570580
self.watch_fd_thread.start()
571581

572-
def _is_master_process(self):
573-
return os.getpid() == self._master_pid
582+
def _is_main_process(self):
583+
return os.getpid() == self._main_pid
574584

575585
def set_parent(self, parent):
576586
"""Set the parent header."""
@@ -674,7 +684,7 @@ def _flush(self):
674684
ident=self.topic,
675685
)
676686

677-
def write(self, string: str) -> Optional[int]: # type:ignore[override]
687+
def write(self, string: str) -> int:
678688
"""Write to current stream after encoding if necessary
679689
680690
Returns
@@ -700,15 +710,15 @@ def write(self, string: str) -> Optional[int]: # type:ignore[override]
700710
msg = "I/O operation on closed file"
701711
raise ValueError(msg)
702712

703-
is_child = not self._is_master_process()
713+
is_child = not self._is_main_process()
704714
# only touch the buffer in the IO thread to avoid races
705715
with self._buffer_lock:
706716
self._buffers[frozenset(parent.items())].write(string)
707717
if is_child:
708718
# mp.Pool cannot be trusted to flush promptly (or ever),
709719
# and this helps.
710720
if self._subprocess_flush_pending:
711-
return None
721+
return 0
712722
self._subprocess_flush_pending = True
713723
# We can not rely on self._io_loop.call_later from a subprocess
714724
self.pub_thread.schedule(self._flush)

0 commit comments

Comments
 (0)