Closed Bug 397039 Opened 17 years ago Closed 17 years ago

Crash downloading file

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: mossop, Assigned: smichaud)

References

Details

(Keywords: crash, regression)

Attachments

(1 file)

From a bugzilla bug, right click on an attachment, do save link as..., file picker appears, enter a filename and hit ok, Minefield crashes almost every time.

http://crash-stats.mozilla.com/report/index/617939ee-6822-11dc-9962-001a4bd43ed6

I believe that this is a regression from bug 395397

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007092004 Minefield/3.0a8pre ID:2007092004
Flags: blocking1.9?
I'm able to reproduce this.

It may be related to bug 396860 -- both happen when a window is closed.

Both are probably old bugs that have been uncovered by my bug 395397
patch.
Assignee: joshmoz → smichaud
Severity: normal → critical
Attached patch FixSplinter Review
Here's a fix for this bug (397039).  Turns out it's not related to bug
396860 ... and it's _really_ wierd.

The crash is caused by [NSApp sendEvent:] trying to process one of the
fake events used by ProcessGeckoEvents() (in nsAppShell.mm) to wake up
the event loop in ProcessNextNativeEvent() (also in nsAppShell.mm).
But the '-1' that the event's 'windowNumber' parameter was originally
set to has been changed into the number of an actual NSWindow object,
and (by chance) that NSWindow object has just been destroyed!

Setting the windowNumber parameter of a fake event to '-1' is a bit
... eccentric.  In the Java Embedding Plugin I've had good luck
setting it to '0'.  I tried that here ... and the crashes went away!
Attachment #281891 - Flags: review?(joshmoz)
Comment on attachment 281891 [details] [diff] [review]
Fix

Looks good, how did you figure this out? Just curious about how you tracked this down.
Attachment #281891 - Flags: superreview?(roc)
Attachment #281891 - Flags: review?(joshmoz)
Attachment #281891 - Flags: review+
> how did you figure this out?

I have a debugging build that (among other things) logs all calls to
[NSApp sendEvent:], and I'm used to seeing ProcessGeckoEvent()'s fake
events in those logs.  So the first part was pretty easy (recognizing
that [NSApp sendEvent:] was choking on a fake event sent to an
NSWindow object that had just been destroyed).

The second part took a little head-scratching (realizing that it might
be wrong to set the fake event's windowNumber to '-1').  But here my
Java Embedding Plugin experience helped.  (In the JEP I use fake
events for exactly the same reason -- to wake up an event loop.  And
I've always set their windowNumber to '0' ... which has always worked
fine.)
I should add that (as best I can tell) setting a fake event's
windowNumber to '0' prevents the OS from ever trying to associate it
with a particular NSWindow object (or trying to send it there).
Attachment #281891 - Flags: superreview?(roc)
Attachment #281891 - Flags: superreview+
Attachment #281891 - Flags: approval1.9+
Landed on trunk.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Looks good
Status: RESOLVED → VERIFIED
Does CFRunLoopWakeUp() not work to wake up the run loop, rather than posting bogus events?
> Does CFRunLoopWakeUp() not work to wake up the run loop, rather than
> posting bogus events?

Not in this case.  [NSApplication nextEventMatchingMask:...] has been
told not to return until it receives an event.
Flags: blocking1.9?
You need to log in before you can comment on or make changes to this bug.