Mac problem - contextual menus cause crash

Scan Tailor specific announcements, releases, workflows, tips, etc. NO FEATURE REQUESTS IN THIS FORUM, please.

Moderator: peterZ

Tulon
Posts: 687
Joined: 03 Oct 2009, 06:13
Number of books owned: 0
Location: London, UK
Contact:

Re: Mac problem - contextual menus cause crash

Post by Tulon »

I took a look at the patch and unfortunately it's very hacky. I would feel bad if I apply it. Also, I believe it might still be possible to crash it by activating a menu item with the Enter key rather than with the mouse.
I downloaded the relevant Qt source code hoping to find a cleaner solution, but the code in question turned out to be quite complex, so no luck on that front. I've noticed
At this point I would actually prefer a solution based on delaying menuAboutToHide(). BTW, it's not hard to make it not crash when menuTriggered() arrives after menuAboutToHide(). It wouldn't be able to actually handle the action, but at least it wouldn't crash. We might even a smart thing and increase the delay in this case.
Or, we may leave it as is. The patch won't be in the official repository, but you can still apply it to your local sources, and I can still link to your build.
Scan Tailor experimental doesn't output 96 DPI images. It's just what your software shows when DPI information is missing. Usually what you get is input DPI times the resolution enhancement factor.
User avatar
n9yty
Posts: 72
Joined: 25 Jul 2010, 22:13

Re: Mac problem - contextual menus cause crash

Post by n9yty »

Very unfortunate situation.

There seems to be no clean fix, because the expectations of ScanTailor do not meet the constraints of the QMenu class with regard to signals and the order they are presented across all supported platforms. I still do not understand the assumptions about the connection type Qt::QueuedConnection and it's "delaying" the signal until after the others are fired... The description says "The slot is invoked when control returns to the event loop of the receiver's thread. The slot is executed in the receiver's thread." If you are in a single-threaded piece of code, or at least if the handler for the slot you are calling is in the same thread as you are, I do not know how this would equate to any sort of delay at all. In fact, the Qt::AutoConnection description says "(default) Same as DirectConnection, if the emitter and receiver are in the same thread. Same as QueuedConnection, if the emitter and receiver are in different threads." which would indicate to my understanding that there is only a difference at all if you are in separate threads. Maybe that is the case here, but the crash log on the Mac seemed to indicate that it was all inside the same thread. But I openly admit I do not have a strong understanding of Qt, and have never used it before, so my understanding is weak at best.

One has to wonder what is so significantly different on platform implementations of Qt that would yield such timing differences between Windows, Linux and Mac. I have not built the code on Linux, so I don't even know if this problem exists there or not, but obviously it doesn't on Windows.

I find it interesting that in the QMenu class they actually go to some lengths to fire the hide event before they process the actual trigger events. I would imagine this is so that any graphical issues are taken care of, when your trigger fires you do not have a menu on the screen any longer.

As for a clean fix in ScanTailor, I don't know for sure how to handle this... Anything else is "hacky" as well... If you catch the event on menuAboutToHide() with some sort of "failsafe" timeout and fire it later if the triggered event never happens. But of course during that time I do not think you could re-invoke the menu. But you can not hold off forever, so finding the timing value that is consistent across all hardware/software systems to deal with relative latencies might prove difficult. But this introduces odd timing issues and more things to go wrong. I tried this initially with your "re-fire" approach, and just inserting a one-second or so delay in there before re-firing and it did seem to work with a lot less "hack" code, but I fear the delay interval would vary too much from end-user to end-user for this to be reliable, so all of the event handlers, while hacky, at least would be consistent.

As for the keyboard menu interaction, I suppose a keypressEvent handler would help there. I hoped actionEvent() would be of use here as well, but I guess it is called after the hide signal is fired as well. But this is just one more hack, so that is probably out as well.

This would handle it, though... I think these are the only three "selecting" keystrokes:

Code: Select all

void ZoneQMenu::keyPressEvent(QKeyEvent *e) {
    if ((e->key() == Qt::Key_Return) ||
        (e->key() == Qt::Key_Enter)  ||
        (e->key() == Qt::Key_Space)
       ) {
        itemWasSelected = this->activeAction() ? true : false;
    }
    QMenu::keyPressEvent(e);
}
I am not going to maintain a separate set of patches to re-apply every time, so I guess there will be no official solution for the Mac userbase. I understand your desire to only have "clean" code and the rejection of "hacky" solutions, so it will mean I guess that the Mac version will not have this function, and unfortunately there is no other way to trigger them except by the pop-up menu, so it is an unfortunately problem.

I understand your stance, but this is, unfortunately, something that happens in open source cross-platform solutions, sometimes it is hard to push changes in that are only required on one platform when it workarounds are not required on others. It makes it frustrating all around.

I still have to go back, in my mind, to the fact that this seems like a design choice in QMenu, which is why it is so difficult to work around cleanly. I would like to understand the whole connection type issue as to why it delays signals on Windows but seems to not do it on the Mac. And, further, I wouldn't ever get that understanding from the parameter by reading the documentation, it seems only to reference behavior with respect to event loops, not the order of signal delivery.

Well, I wanted to help, but since I still do not have a scanner built, and may not get around to it for the foreseeable future, I have already poured considerable more time into this for a project I am not using than is probably reasonable.

So, I guess consider this closed and the problem will remain until there is a solution you are comfortable with integrating. I don't think I have anything more to offer on this front.
Tulon
Posts: 687
Joined: 03 Oct 2009, 06:13
Number of books owned: 0
Location: London, UK
Contact:

Re: Mac problem - contextual menus cause crash

Post by Tulon »

n9yty,

Don't give up that quickly. Both maintaining an unofficial patch and going with delaying of menuAboutToHide() seem like fine options to me. I once maintained an unofficial patch for Qt itself, which fixed a crash in their drawing code which took them like a year to acknowledge and fix. I don't expect any problems with delaying of menuAboutToHide() either, and here is why:

On Windows and Linux, even a zero delay is enough. A zero delay will just move the slot invocation at least one iteration of main event loop forward. The main event loop in Qt or any other GUI Toolkit looks like this:

Code: Select all

for (;;) {
    Event* ev = waitForEvent();
    if (event->type == Event::Quit) {
        break;
    } else if (ev->type == Event::MousePressed) {
        ...
    } else if ...
}
If a slot is on a queued connection, it won't be invoked immedately, but instead a certain event will be enqueued, which will eventually get picked up by the main event loop.

So, in case of Windows and Linux the code on the Qt side probably like this:

Code: Select all

void QMenu::activated()
{
    emit aboutToHide();
    emit triggered();
}
While on Mac OS X it might look like this:

Code: Select all

void QMenu::activated()
    emit aboutToHide();
    startFadeOutSequence();
}

void QMenu::fadeOutFinished()
{
   emit triggered();
}
So, if we delay aboutToHide() handler for a longer period than fade-out animation or whatever if causing a delay on the Qt side, we are fine. Again, we might automatically adjust the delay if we do get menuItemTriggered() after menuAboutToHide(). And no, system load won't affect this, as two events sheduled for different time slots would be executed in the correct order no matter what.
Scan Tailor experimental doesn't output 96 DPI images. It's just what your software shows when DPI information is missing. Usually what you get is input DPI times the resolution enhancement factor.
User avatar
n9yty
Posts: 72
Joined: 25 Jul 2010, 22:13

Re: Mac problem - contextual menus cause crash

Post by n9yty »

Tulon wrote:So, if we delay aboutToHide() handler for a longer period than fade-out animation or whatever if causing a delay on the Qt side, we are fine. Again, we might automatically adjust the delay if we do get menuItemTriggered() after menuAboutToHide(). And no, system load won't affect this, as two events sheduled for different time slots would be executed in the correct order no matter what.
My mind is clearly not firing all synapses today... I had thought about it, but am unsure how you know, in any case, whether or not triggered was selected when you receive your "aboutToHide" signal so you know whether you need to delay it or not. Assuming you can not, which I think is the case, then you delay all of them for a set time.

Since this is time and not event loop cycles, I believe it would be dependent on machine load and performance because it would eat a different number of cycles in a given amount of time.

So, how to know in aboutToHide() to keep delaying without knowing if triggered() was sent at all? And in triggered, how to know about aboutToHide()? Since aboutToHide() is seemingly always firing first per QMenu design, I suppose the *Interaction class can store that so we will know at the time triggered() arrives.

But at some point an assumption has to be made about receiving the aboutToHide() signal and how long to wait in any case to decide if a triggered() event is coming or not. And since the aboutToHide() is removing the *Interaction object, if the trigger does fire later it will crash as there is nothing to receive the signal, right? Or I am missing something, I am not looking at the code right now so that is possible. :)

I did think this through briefly before all the other chicanery with the QMenu subclass, but in the end wasn't sure I had anything more reliable. And as "hacky" as the QMenu subclass is, it does catch the valid information to send a signal only when the menu goes away and provide information about if it was triggered or not at that point. So, if you have a brief outline of changes to make as you suggest, I would be happy to try to implement them.
Tulon
Posts: 687
Joined: 03 Oct 2009, 06:13
Number of books owned: 0
Location: London, UK
Contact:

Re: Mac problem - contextual menus cause crash

Post by Tulon »

n9yty wrote:And since the aboutToHide() is removing the *Interaction object, if the trigger does fire later it will crash as there is nothing to receive the signal, right?
This got me thinking. If the *Interaction object was already destroyed by the time triggered() signal was dispatched, there would be no crash, as Qt would automatically disconnect our slots in this case and the signal would just not be delivered. So, either the object is not yet destroyed at that time, or there is a bug in Qt. Now, the first case could only happen if there is no delay between dispatching aboutToHide() and triggered(). The destruction of *Interaction object is itself delayed, but only until the next iteration of the event loop. Basically if there is a delay between aboutToHide() and triggered(), then the latter wouldn't be delivered, so no crash, an if there is no delay, then putting aboutToHide() on a queued connection would work, just as it does for Windows and Linux.
To better understand what's going on, could you log the following events, together with times, on the original source code:
* menuItemTriggered()
* menuAboutToHide()
* Destructor of ZoneContextMenuInteraction
Scan Tailor experimental doesn't output 96 DPI images. It's just what your software shows when DPI information is missing. Usually what you get is input DPI times the resolution enhancement factor.
User avatar
n9yty
Posts: 72
Joined: 25 Jul 2010, 22:13

Re: Mac problem - contextual menus cause crash

Post by n9yty »

Tulon wrote:
n9yty wrote:And since the aboutToHide() is removing the *Interaction object, if the trigger does fire later it will crash as there is nothing to receive the signal, right?
This got me thinking. If the *Interaction object was already destroyed by the time triggered() signal was dispatched, there would be no crash, as Qt would automatically disconnect our slots in this case and the signal would just not be delivered.
Except for the fact that the connect is done to a boost encapsulated object in the QtSignalForwarder class, which is not destroyed by menuAboutToHide(), or is it? Again, I'm not on strong ground here because I am not fully versed in all of the things in boost::bind, but I am assuming that the intention is like a lambda to capture the object in it's state at the time it is bound, but if you did try to call that when the actual object was gone, wouldn't that cause the crash?

At any rate, I will look at your other questions and get back to you with some timing information.

OK - here:

Code: Select all

menuAboutToHide Called: Wed Nov  3 15:51:14 2010
time(): 1288817474
ZoneContextMenuInteraction Destroyed: Wed Nov  3 15:51:14 2010
time(): 1288817474
Bus error
So menuAboutToHide() fires, and immediately destroys the *Interaction object. menuItemTriggered() is not called before the crash:

Code: Select all

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   QtGui                         	0x00e25b92 QMenu::findIdForAction(QAction*) const + 18
1   QtGui                         	0x00e26f06 QMenuPrivate::activateCausedStack(QList<QPointer<QWidget> > const&, QAction*, QAction::ActionEvent, bool) + 70
2   QtGui                         	0x00e2d968 QMenuPrivate::activateAction(QAction*, QAction::ActionEvent, bool) + 488
3   QtGui                         	0x00e3022e QMenu::mouseReleaseEvent(QMouseEvent*) + 254
4   QtGui                         	0x009e2bd3 QWidget::event(QEvent*) + 4323
5   QtGui                         	0x00e2b7ef QMenu::event(QEvent*) + 95
6   QtGui                         	0x009899ff QApplicationPrivate::notify_helper(QObject*, QEvent*) + 175
7   QtGui                         	0x0098a4f9 QApplication::notify(QObject*, QEvent*) + 2681
8   QtCore                        	0x015db4f2 QCoreApplication::notifyInternal(QObject*, QEvent*) + 98
9   QtGui                         	0x0091b358 QApplicationPrivate::globalEventProcessor(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 7944
10  com.apple.HIToolbox           	0x904aeeef DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1567
11  com.apple.HIToolbox           	0x904ae1b6 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 411
12  com.apple.HIToolbox           	0x904d097b SendEventToEventTarget + 52
13  com.apple.HIToolbox           	0x904e2497 ToolboxEventDispatcherHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 1257
14  com.apple.HIToolbox           	0x904af340 DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 2672
15  com.apple.HIToolbox           	0x904ae1b6 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 411
16  com.apple.HIToolbox           	0x904d097b SendEventToEventTarget + 52
17  QtGui                         	0x00932ca4 QEventDispatcherMac::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 596
18  QtCore                        	0x016bc041 QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 65
19  QtCore                        	0x016bc27d QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 189
20  QtCore                        	0x016be57e QCoreApplication::exec() + 174
21  scantailor                    	0x00007121 main + 1981 (main.cpp:184)
22  scantailor                    	0x0000672e _start + 216
23  scantailor                    	0x00006655 start + 41
Is this because QMenu is storing the actions before it calls aboutToHide() and doesn't check their state again before it tries to fire them? I'm not sure. I also did try to put a similar time logging statement in the QtSignalForwarder::handleSignal() method, but it was never called. Re-examining the crash logs at the start of this thread show that Qt isn't always crashing in exactly the same way, but it is always related to trying to handle the action event.

I put a timestamp log statement in the destructor of QtSignalForwarder, and it is destroyed at the same time as the *Interaction object:

Code: Select all

menuAboutToHide Called: Wed Nov  3 16:08:31 2010
time = 1288818511.928
ZoneContextMenuInteraction Destroyed: Wed Nov  3 16:08:31 2010
time = 1288818511.954
QtSignalForwarder Destroyed: Wed Nov  3 16:08:31 2010
time = 1288818511.954
QtSignalForwarder Destroyed: Wed Nov  3 16:08:31 2010
time = 1288818511.954
Bus error
Last edited by n9yty on 03 Nov 2010, 17:09, edited 1 time in total.
Tulon
Posts: 687
Joined: 03 Oct 2009, 06:13
Number of books owned: 0
Location: London, UK
Contact:

Re: Mac problem - contextual menus cause crash

Post by Tulon »

n9yty wrote:Except for the fact that the connect is done to a boost encapsulated object in the QtSignalForwarder class, which is not destroyed by menuAboutToHide(), or is it?
QtSignalForwarder becomes a child of its first argument which is a QAction. Those QAction's are owned by a QMenu, which is owned by ZoneContextMenuInteraction. So, when ZoneContextMenuInteraction is destroyed, QtSignalForwarder gets destroyed as well, breaking any connections gracefully. And BTW, QtSignalForwarder is only used for triggered(), not for aboutToHide(). As triggered() comes first, ZoneContextMenuInteraction is guaranteed to still be alive.
Scan Tailor experimental doesn't output 96 DPI images. It's just what your software shows when DPI information is missing. Usually what you get is input DPI times the resolution enhancement factor.
User avatar
n9yty
Posts: 72
Joined: 25 Jul 2010, 22:13

Re: Mac problem - contextual menus cause crash

Post by n9yty »

Tulon wrote:As triggered() comes first, ZoneContextMenuInteraction is guaranteed to still be alive.
But on the Mac, triggered() does not come first. :)
User avatar
n9yty
Posts: 72
Joined: 25 Jul 2010, 22:13

Re: Mac problem - contextual menus cause crash

Post by n9yty »

Back to the delaying of menuAboutToHide()....

Using your suggestion:

Code: Select all

if (m_extraDelaysDone < 500) {
  ++m_extraDelaysDone;
  QTimer::singleShot(0, this, SLOT(menuAboutToHide()));
  return;
}
I kept expanding the iterations it would re-schedule the signal... When I tried 500, I finally got it to fire triggered()... But it was not reliable at that point, as it seems to take more time on some invocations than on others....

Code: Select all

menuAboutToHide Called: Wed Nov  3 16:23:48 2010
time = 1288819428.831
Iteration: 1
menuAboutToHide Called: Wed Nov  3 16:23:48 2010
time = 1288819428.837
Iteration: 2
 .
 .
 .
menuAboutToHide Called: Wed Nov  3 16:23:48 2010
time = 1288819428.956
Iteration: 491
QtSignalForwarder::handleSignal() Called: Wed Nov  3 16:23:48 2010
time = 1288819428.956
menuItemTriggered Called: Wed Nov  3 16:23:48 2010
time = 1288819428.956
menuAboutToHide Called: Wed Nov  3 16:23:49 2010
time = 1288819429.206
Iteration: 492
 .
 .
 .
menuAboutToHide Called: Wed Nov  3 16:23:49 2010
time = 1288819429.324
Iteration: 500
menuAboutToHide Called: Wed Nov  3 16:23:49 2010
time = 1288819429.324
ZoneContextMenuInteraction Destroyed: Wed Nov  3 16:23:50 2010
time = 1288819430.702
QtSignalForwarder Destroyed: Wed Nov  3 16:23:50 2010
time = 1288819430.702
QtSignalForwarder Destroyed: Wed Nov  3 16:23:50 2010
time = 1288819430.702
I went back to when I had changed the QTimer::oneShot call use a delay of 1000 instead of 0, and at least on this dev machine it seems "stable", but if you bring up the menu and dismiss it, you can't re-invoke it until that time has expired. Small price to pay if it works reliably, though, I just wonder about slower computers and what time may be required to allow the triggered() signal to fire and process.
Tulon
Posts: 687
Joined: 03 Oct 2009, 06:13
Number of books owned: 0
Location: London, UK
Contact:

Re: Mac problem - contextual menus cause crash

Post by Tulon »

n9yty wrote:
Tulon wrote:As triggered() comes first, ZoneContextMenuInteraction is guaranteed to still be alive.
But on the Mac, triggered() does not come first. :)
Oops, my mistake. My first point still holds though.
n9yty wrote:I kept expanding the iterations it would re-schedule the signal... When I tried 500, I finally got it to fire triggered()...
If you need more than one or two iterations, you don't do it like that - you specify a real timeout.

The log you produced is quite interesting. aboutToHide() was fired first and issued deleteLater() on ZoneContextMenuInteraction. 125 ms after that, triggered() was fired. Fine - could be some animation or something. 746 ms later, ZoneContextMenuInteraction finally gets destroyed. What took it so long? This should have happened right at the next main loop iteration, and we saw almost 500 of them pass by.

Anyway, I believe that if you delay menuAboutToHide() by say 200 ms, that will solve the problem. Performance shouldn't be an issue, as I suspect that 125 ms delay was done the very same way you are going to delay menuAboutToHide(). If so, no matter the performance, those will be fired in the correct order.
Scan Tailor experimental doesn't output 96 DPI images. It's just what your software shows when DPI information is missing. Usually what you get is input DPI times the resolution enhancement factor.
Post Reply