Connectome tool: Segmentation fault when loading a node image on MacOSX #298
Comments
|
Seems related to what we're currently discussing on #295...? |
|
Yep: @chunhungyeh What's your QT version? I'm wondering if this has been introduced in QT 5.5.0 |
|
Maybe. Other option is that variable is now declared thread-local. Which would make sense since it really shouldn't be touched from other threads... |
|
@Lestropie : I am using QT 5.4.2. |
As discussed in #293, #295, #296 & #298. This essentially ensures the ProgressBar display only kicks in after 1s (so will only affect things for large loads); that the main window does not try to process further updates in the meantime; that mouse events to the main window are not processed until completion; and that the GL context is preserved only if one in found. This works as expected on Qt4. On Qt5, for some reason the background of the main window is cleared to default (grey) as soon as the progressbar is displayed. I'm hoping this is a glitch in Qt5.5 - I can't see any reason for them to clear the background of a widget that explicitly has updates disabled...
|
please try with the latest commit on master (73b21ad), see if that fixes it... |
|
The latest commit has fixed the issue. 👍 |
|
Not quite: get this following 73b21ad: |
|
Crap, I was hoping this wouldn't happen... I think what's going on here is that Qt's whole cross-thread event handling only works if all participating threads are started using their own QThread backend. I have no idea how to get around that - other than fantastical solutions like starting another Qthread waiting on a condition variable, forwarding any updates to the main event loop as soon as one of the worker thread notifies it... |
|
Ah, spoke too soon. From the Qt docs:
👍 |
|
@Lestropie, do you want to try the above solution? Probably need to add the |
|
Sorry, wrong suggestion. Best place is 3 |
|
Actually, it was just simpler to pass a |
|
No good:
Have removed all multi-threaded elements from the connectome tool and reverted the progress dialog code; if we want to play with this further it should happen in another branch. |
|
Just to confirm: the dialog didn't even appear until processing was done, right? It's not that it appeared, did nothing, then disappeared again? In that case, the issue is most likely that the backend being invoked is a method of the |
Yep. Figured it out using |
|
OK, I think I've finally got a handle on what's going on. I've made a few changes to the progress dialog along the lines of what I was suggesting, but that won't fix it. It's all in the So the real issue is that the Probably the easiest thing to do is actually to run any function that might involve multithreading within a dedicated QThread, so the main event loop is free to run. I've just pushed an example of that to the |
|
Oh man, this thing is doing my head in. So in terms of connectome tool stuff, I ended up removing stuff where I was explicitly using a ProgressBar with multi-threading; these previously had to do with determining meshes & exemplars, which are now done outside of I had a go at integrating this idea. Got the So I think that although this works, I won't actually use it in this particular case; I'll just |
|
Good news that you managed to get the progress bar to work! But there shouldn't be any problem with performing the rest of the processing within the UI thread: you would have set up a slot to receive the thread's |
|
It could certainly be done; it's just a lot of code ballet to enable a progress dialog on something that in a lot of cases doesn't even run long enough to activate the dialog window anyways. Also, completion of the progress dialog would re-enable GUI updates, even though the slot function that completes the tool initialisation isn't guaranteed to have finished, so you might have to handle that. I'm happy to keep the changes if you are, I just don't think I'll use it currently. The main heavy lifting in the connectome tool that used to need a progress dialog is now done before the fact in other commands. |
|
Totally agree this approach would only be worthwhile if the processing is expected to take a seriously long time, the progressbar doesn't even pop up until one second into processing anyway... In terms of updates to the GUI after the progressbar completes, that could indeed be a problem. We'll need to worry about that if & when we decide to use this approach. For now, I think I might revert to a simpler approach that doesn't allow for multi-threading, and check that any progressbar updates are actually running within the GUI thread - might help with debugging in future... |
The QMetaObject::invokeMethod() call now uses a Qt::DirectConnection, which won't work if the invoking thread is not the GUI thread. This should at least alert developers that using a progressbar in a multi-threaded context is not supported. See #298 for details.
|
OK, just pushed a few changes I'd made on the |
|
@chunhungyeh Are you able to test this using fully up-to-date code? |
|
Yes. The connectome tool is working nicely on my laptop after updating the codes. 👍 |
MRview crashed when I loaded a node image with the connectome tool.
Here are the problem details, hopefully they are useful...:
The text was updated successfully, but these errors were encountered: