Make image showing functions thread-safer #211
Conversation
Notes from Claude.ai:
I've made the ArrayViewer class thread-safe by implementing the
following changes:
1. Thread-safe Constructor
(src/ndv/controllers/_array_viewer.py:100-108)
- Moved GUI component initialization to a separate
_init_gui_components() method.
- Queue GUI operations to the main thread using
_app.ndv_app().call_in_main_thread().
- Block until operations complete to maintain synchronous behavior.
2. Thread-safe View Synchronization
(src/ndv/controllers/_array_viewer.py:106-108)
- Queue _fully_synchronize_view() to main thread since it contains
GUI operations like create_sliders().
3. Thread-safe Show/Hide/Close Methods
(src/ndv/controllers/_array_viewer.py:216-233)
- Modified show(), hide(), and close() methods to queue
set_visible() calls to main thread.
- Maintain synchronous behavior by blocking until operations
complete.
The fix ensures that all GUI-sensitive operations are properly queued
to the main thread while maintaining the existing synchronous API.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #211 +/- ##
==========================================
- Coverage 85.92% 85.90% -0.02%
==========================================
Files 46 46
Lines 5202 5209 +7
==========================================
+ Hits 4470 4475 +5
- Misses 732 734 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
having a look into this today. (Context, for anyone reading this, is trying to get ndv and other Qt/python apps working with Fiji's new experimental python runtime support) As I played around with this a bit, it does work... but I have to say, more generally, that the model of running everything on a non-main thread is going to be so unusual to most libraries, that I think this might be more of a "core" fiji problem than we want it to be. I don't imagine it will work well to have to tell every library anyone wants to use in Fiji to use something like the So, let's keep playing with this (and other patterns) in the context of Fiji's python launcher |
|
My thinking is that in both Python and Java worlds, programmers rarely if ever expect to see a native crash—only exceptions. If we decide not to make I agree with what you're saying about ndv not being the only library that's going to be tougher to invoke from Fiji. This is also an issue in Java, where you have to queue GUI-manipulating functions to the EDT via |
sys.excepthookwith this phrasing, i just want to double check here that you know about the whole
so, if you're going to "own" the main event loop, and if you're going to commit to using Qt's event loop, then fiji.py should also probably install a custom except hook. app.exec()I think it's also worth discussing the call to That might be a bit too presumptuous if you want to more generally support python in Fiji. There are many different event loop models in python, plenty of event-loop requiring python modules don't use Qt. (the main alternates being asyncio/anyio/trio and other structured concurrency libs). If you call So... Fiji might also try having some magic method for picking an event loop and going back and forth? ... but it's possibly that AWT/Swing will make this situation harder here than it is over in IPython... I tried playing around a bit briefly with alternatives to calling I will say that Qt apps are noticeably slower/laggier on IPython anyway, just some thoughts. will keep playing when I have a moment! |
|
Thanks @tlambert03, I didn't know about I tried adding a # Install custom exception handler to prevent Qt crashes from
# unhandled Python exceptions. This is important because Qt's
# default behavior is to call qFatal and crash the app.
original_excepthook = sys.excepthook
def fiji_excepthook(exc_type, exc_value, exc_traceback):
# Log the exception.
_logger.error(
"Unhandled exception:",
exc_info=(exc_type, exc_value, exc_traceback),
)
# Call original handler (prints to stderr).
original_excepthook(exc_type, exc_value, exc_traceback)
sys.excepthook = fiji_excepthookbut it still crashes when Qt operations happen on the wrong thread. uncaught NSException crashHere is what Claude said about it:
Pushing Claude a bit more, we tried some other approaches:
Here's what I ended up with: https://github.com/fiji/fiji/blob/f1460d8d01d092fe93d0ffc325db469c155af538/config/fiji.py Still crashes when you forget the |
|
Very interesting -- I didn't know that the C++ terminate handler gets called for ObjC exceptions (I guess it makes sense given the shared Itanium ABI).
One thing to keep in mind is that The default terminate handler (depending on the platform, but definitely on macOS) prints some useful information, like the type and message of the uncaught exception, if that was the cause, or whatever else the cause was. So it might be good to call the default terminate handler instead of (or before) just exiting (and btw
I'd also note that the ObjC So it might be nice to print a hint that GUI threading could be a possible cause, but maybe not sound too definite, lest it mislead the reader. I also don't know what the chances are of the Python interpreter being in a consistent state when the terminate handler is called. If Probably best if production terminate handlers are implemented in C or C++ and kept minimal so that nothing futher can go wrong. |
3 participants
This patch improves ndv's image showing functions to work more seamlessly when called from non-main threads, especially on macOS.
Below is an MCVE script that illustrates the problem before this patch. Without this patch, commenting out the
@ensure_main_threaddecorator causes the program to crash, whereas with this patch, the program works with or without the decorator in place.Note that the patch itself was written by Claude.ai, not me, so I won't be offended by any harsh criticism. 😆