Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[btkaos]

btkaos, I see, but I was refering to the patch, not to the thread. For a patch fixing such a serious issue, it is very, lets say, economic.

Ok, sorry for the confusion.

Is it possible to test for an empty notebook in the event handler and only ignore it in this case? That would probably not break the behaviour.

I think this is not possible to do in a safe way, given that we have two (implicit) event queues. It could well happen that you do the test and the page is alive, but the delete event is in the queue waiting for delivery.

I think that the solution is acceptable, but not the way it is posted. It must be well documented in the patch. Also, beeing a GTK+ only problem means it should be properly #ifdef'd to this platform, because if it breaks functionality, we do not want it to happen on Mac for example. And please, if someone pushes the fix to GTK+, do get in touch with us when it gets accepted, so that we can add the proper build dependency on the GTK+ version.

I think fixing this properly in aMule is futile, let's hope the GTK+ people react.

[Stu Redman]

For a patch fixing such a serious issue, it is very, lets say, economic.

Many serious problems can be fixed with very economic patches. 
Seriously, this was just an idea I wanted to try: if GTK crashes on the mouse-up event following the deletion of the page, well, then just catch it in the app and don't send it on. And it appears to work out. I can easily fine-tune it a bit so that the resulting notebook problems vanish. (And add the ifdef). After Christmas. 

I think this is not possible to do in a safe way, given that we have two (implicit) event queues. It could well happen that you do the test and the page is alive, but the delete event is in the queue waiting for delivery.

I see no problem there. We have a trigger right before the page gets deleted. Just swallow up the next mouse-up within a short time period, and we should be save.

• When releasing the right button, the contextual menu doesn't disapear.

That's so without the patch too. (Ubuntu Intrepid.)

Btw: can you descripe the click-timing required to crash it ? I'd like to see it myself for once.

[Stu Redman]

Here's the slightly more sophisticated version of the patch, please test. Should filter only the events that could choke GTK.
Did anybody ever get a crash on closing a tab through the context menu?

[btkaos]

Here's the slightly more sophisticated version of the patch, please test. Should filter only the events that could choke GTK.
Did anybody ever get a crash on closing a tab through the context menu?

Thanks a lot STU Redman, this patch looks much better (as ugly as the workaround is). Let me think about it and test it, there come busy days so please be patient.

I'd really like more testers to be sure this is the cause. Both previous patches fix the bug for me and for Luca. legolas, did you make any progress? There are more people in the Ubuntu bug-tracker (https://bugs.launchpad.net/ubuntu/+source/amule/+bug/59138) who can reproduce it, let's see if someone tries the patch.

To reproduce the bug: First make aMule work a lot. This ideally involves having 40+ downloads with a non-optimized build.
Then open at least 3 or 4 tabs in search, and start closing them quickly using the 'x' button. Always close the leftmost tab, until there's none. Try until you crash aMule. This works for me about 33% of times.

[btkaos]

My 10 cents: I've tried Stu's patch (the first one) and the workaround works on Fedora 10 too. No time to test for side-effects so far.

Glad to hear so. I'll try to get GTK+ fixed upstream.

Meanwhile if a workaround in aMule is convenient, great, I'll inform you guys when GTK+ is patched.

[GonoszTopi]

I'd much more like to close the notebook tab on the mouse_release event, which would only involve changing the binding of the event handler from EVT_LEFT_DOWN to EVT_LEFT_UP (only for the close event generator, otherwise process EVT_LEFT_DOWN as normal).

[GonoszTopi]

Ok, I created the patch to accomplish the task mentioned in my previous post. Unfortunately I cannot reproduce the crash so would you be as kind as to test it?

[Stu Redman]

That's of course the best solution. 

[Stu Redman]

When I add a little CPU stress I can easily reproduce it (crashing on last close):

Code: [Select]

if ((tab != -1) &&  (flags == wxNB_HITTEST_ONICON)) {
// User did click on a 'x'
DeletePage(tab);
for (double f = 0.0; f < 1.0; f += 0.00000001);
} else {
// Is not a 'x'. Send this event up.
event.Skip();
}
With GonoszTopi's patch it's fine.

[btkaos]

I'd much more like to close the notebook tab on the mouse_release event, which would only involve changing the binding of the event handler from EVT_LEFT_DOWN to EVT_LEFT_UP (only for the close event generator, otherwise process EVT_LEFT_DOWN as normal).

Yes I was trying to figure out why aMule is the only app bitten by this GTK+ bug, and the reason is every other app closes a page when going up.

So if we change MuleNotebook behaviour the problem hopefully solved without having to wait for the GTK+ guys.

Edit Stu: fixed broken quote

[GonoszTopi]

Yes I was trying to figure out why aMule is the only app bitten by this GTK+ bug, and the reason is every other app closes a page when going up.

The "standard" behaviour for close buttons requires that both the click and the release event happens on the same close button. The patch only checks that a left-release event happens on the 'x', but I think it's enough if it works around the bug in GTK.

[btkaos]

The "standard" behaviour for close buttons requires that both the click and the release event happens on the same close button. The patch only checks that a left-release event happens on the 'x', but I think it's enough if it works around the bug in GTK.

Well, this behavior allows the user to think twice and cancel the closing by moving the pointer away of the x while the button is pressed. Certainly, checking that the pointer is on the x when button release happens will make MuleNotebook more conformant with what seems to be standart GTK+ behavior.

However, I don't know what is expected in other toolkits.

[phoenix]

For the record, it also sometimes crashes when you close the last notepad in the chat window.

[Stu Redman]

With SVN >= 9293 ?