ipykernel icon indicating copy to clipboard operation
ipykernel copied to clipboard

Fix OutStream hooks not being called

Open steovd opened this issue 10 months ago • 4 comments

#1110 introduced OutStream hooks, which are stored in a thread-local variable and invoked in _flush. However, since _flush usually runs in a background thread, the hooks only work when registered from this thread.

This PR makes it so that the hooks are accessed from the thread which calls flush, which I assume is the expected behavior.

steovd avatar Feb 27 '25 10:02 steovd

Thanks @steovd, do you think this can be related to #1369 (comment)?

I don't think either this patch or #1110 have anything to do with it. The same happens on main even when I revert the only possibly relevant change from #1110. I'm not that familiar with the code though, so I could be wrong.

steovd avatar Feb 27 '25 11:02 steovd

Just checking in - any update on this? This fix would resolve a frustrating bug I'm currently facing. Thanks!

nathanmcavoy avatar Jul 15 '25 10:07 nathanmcavoy

Just checking in - any update on this? This fix would resolve a frustrating bug I'm currently facing. Thanks!

You can work around this bug by registering your hooks from the background thread. I don't have the code at hand but I think it went something like this:

outstream.pub_thread.schedule(functools.partial(outstream.register_hook, myhook))

steovd avatar Jul 15 '25 13:07 steovd

Just checking in - any update on this? This fix would resolve a frustrating bug I'm currently facing. Thanks!

You can work around this bug by registering your hooks from the background thread. I don't have the code at hand but I think it went something like this:

outstream.pub_thread.schedule(functools.partial(outstream.register_hook, myhook))

Thanks! I ended up doing something similar, but looks like your version is neater.

Adding the hook itself is already a hacky fix to what I am trying to do so I still think this PR is useful, but glad there is a workaround.

nathanmcavoy avatar Jul 15 '25 14:07 nathanmcavoy