331088 - crash with <input type=file> and events

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[georgi - hopefully not receiving bugspam]

crash with <input type=file> and events

changing the type of input from file to text while the "file open" dialog
is open causes crash with signs of memory corruption.
the top of stack is NS_RELEASE and events are involved in the stack.

affects ff and seamonkey trunk.

ff 1.5 seems safe.

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

No problem on windows, because of bug 100180.

Comment 3

[georgi - hopefully not receiving bugspam]

macosx seems safe

Comment 4

[georgi - hopefully not receiving bugspam]

bclary, i don't crash anymore neither on trunk nor or branches, so it seems fixed by something.

can you verify it and resolve per your opinion?

Comment 5

[Alfred Peng]

Attachment: core stack

I still got the crash with the testcase on Solaris10.

Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20060901 Minefield/3.0a1

Comment 6

[georgi - hopefully not receiving bugspam]

a simpler testcase still crashes with scary stack:

#5  <signal handler called>
#6  0xb73bbae7 in strstr () from /lib/i686/libc.so.6
#7  0xb7337e12 in XRenderFindDisplay () from /usr/lib/libXrender.so.1
#8  0xb733482a in XRenderFreeGlyphs () from /usr/lib/libXrender.so.1
#9  0xb77bc1e0 in cairo_xlib_surface_get_display () from /usr/lib/libcairo.so.2
#10 0xb779b2a7 in cairo_scaled_font_get_font_matrix ()
   from /usr/lib/libcairo.so.2
#11 0xb77907ef in cairo_create () from /usr/lib/libcairo.so.2
#12 0xb779095f in cairo_create () from /usr/lib/libcairo.so.2
#13 0xb779b99b in cairo_scaled_font_get_font_matrix ()
   from /usr/lib/libcairo.so.2
#14 0xb779bce4 in cairo_scaled_font_destroy () from /usr/lib/libcairo.so.2
#15 0xb7791c61 in cairo_font_face_status () from /usr/lib/libcairo.so.2
#16 0xb779185d in cairo_debug_reset_static_data () from /usr/lib/libcairo.so.2
#17 0xb7f2a79b in MOZ_gdk_display_close (display=0x81540e8)
    at /opt/joro/firefox-cvs/mozilla/toolkit/xre/nsAppRunner.cpp:2371
#18 0xb7f31bca in XRE_main (argc=1, argv=0xbf937044, aAppData=0x8111380)
---Type <return> to continue, or q <return> to quit---q
 at /optQuit
(gdb) frame 6
#6  0xb73bbae7 in strstr () from /lib/i686/libc.so.6
(gdb) x/i $pc
0xb73bbae7 <strstr+39>: movzbl (%edx),%eax
(gdb) p/x $edx
$1 = 0x5a5a5a5a
(gdb) 

Comment 7

[georgi - hopefully not receiving bugspam]

Attachment: xul12-crash.xul - try typing in the input box

Comment 8

[Martijn Wargers (dead)]

This also seems to crash on windows with the patch from Mats.

Comment 9

[georgi - hopefully not receiving bugspam]

btw, sometimes the crash is delayed or on exit

Comment 10

[Martijn Wargers (dead)]

Hmm, it doesn't seem to crash in a regular build, only in a debug build, for some reason.

Comment 11

[Mats Palmgren (inactive)]

The stack in comment 6 looks similar to the problem I had in bug 417163
with the Qt code trying to use Display after it was closed and deallocated.
It should go away if you  disable jemalloc in your .mozconfig
or comment out the "display_close" code in toolkit/xre/nsAppRunner.cpp.

It is most likely a different issue than the original crash, which also
occurs on Windows per comment 8.  FWIW, I can't reproduce the crash
on Linux with either testcase.

Comment 12

[Brandon Sterne (:bsterne)]

I'm not able to reproduce the crash on my Linux debug build either.  I do get the following console error, though:

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSHTMLInputElement.setSelectionRange]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/content/bindings/textbox.xml :: onxblmousedown :: line 271"  data: no]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008070315 Minefield/3.1a1pre

Comment 13

[Brandon Sterne (:bsterne)]

I'm also not able to reproduce on my Mac trunk debug build.  The first testcase only hangs for me and I am not able to get any useful stack contents.  Damon, who could we get to take a look at this?

Comment 14

[Brandon Sterne (:bsterne)]

Attachment: Mac backtrace

I take that back.  Here's my backtrace from the Mac hang, but it looks quite different from the previous stacks posted... not sure if this is helpful.

Comment 15

[georgi - hopefully not receiving bugspam]

i don't seem to crash on linux trunk. such modal dialogs/events may be related to Bug 441505

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In attachment #215647 [details], we have a busy hang because in each call to nsThread::ProcessNextEvent from nsXULWindow::ShowModal, nsBaseAppShell::OnProcessNextEvent is dispatching a dummy event which is then processed (doing nothing). So nsXULWindow::ShowModal is in a busy infinite loop.

However, events are still being processed so I think this wouldn't be fatal except both the alert and the file dialog seem to be disabled. That I think is the core bug here, so CC'ing Stephen. However, this doesn't seem to have any security implications.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment 305510 [details] does not do anything for me on Mac trunk debug. Is it still relevant?

Comment 18

[georgi - hopefully not receiving bugspam]

at the time of filing this, as in description:
>top of stack is NS_RELEASE and events are involved in the stack

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Which attachment are you talking about? Crashes around NS_RELEASE are bad, but attachment #215647 [details] isn't a crash (anymore?).

Comment 20

[georgi - hopefully not receiving bugspam]

at the time of filing Attachment 215647 [details] crashed as in the description.

Comment 21

[georgi - hopefully not receiving bugspam]

 Attachment 215647 [details] (named testcase) still crashes but needs more steps to reproduce.

on linux x86_64:
1. open the attachment
2 [review]. click in the textfield
3. wait for alert('done') and click 'ok' in the alert
4. in the shown filedialog open a file
5. crash with stack:
#3  0x0000000000460348 in nsProfileLock::FatalSignalHandler (signo=11)
    at nsProfileLock.cpp:216
#4  <signal handler called>
#5  0x000000000092228c in nsTextControlFrame::GetFireChangeEventState (
    this=0xdddddddddddddddd)
    at /opt/joro/firefox-central/src/layout/forms/nsTextControlFrame.h:211
#6  0x000000000092107b in nsFileControlFrame::MouseClick (this=0x2aaaaec0d3e8, 
    aMouseEvent=0x2aaaae6a61c0)
    at /opt/joro/firefox-central/src/layout/forms/nsFileControlFrame.cpp:366
#7  0x000000000092123a in nsFileControlFrame::MouseListener::MouseClick (
    this=0x2aaaaebf2f80, aMouseEvent=0x2aaaae6a61c0)
    at /opt/joro/firefox-central/src/layout/forms/nsFileControlFrame.cpp:623
#8  0x0000000000afb2e0 in DispatchToInterface (aEvent=0x2aaaae6a61c0, 
    aListener=0x2aaaaebf2f80, aMethod=<error reading variable>, 
    aIID=@0x161a9e0)
    at /opt/joro/firefox-central/src/content/events/src/nsEventListenerManager.cpp:184
#9  0x0000000000afdc65 in nsEventListenerManager::HandleEvent (
    this=0x2aaaadeb1a80, aPresContext=0x2aaaae4a5400, aEvent=0x7fff8d50ef00, 
    aDOMEvent=0x7fff8d50eca0, aCurrentTarget=0x2aaaaeb78520, aFlags=514,

Comment 22

[georgi - hopefully not receiving bugspam]

i mean it crashes on today trunk from central

Comment 23

[georgi - hopefully not receiving bugspam]

the instructions in comment #21 crash 3.0-latest with same stack

Comment 24

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #16)

Roc, your hang (which I can reproduce) is a dup of bug 436473 and bug
442442, and seems unrelated to this bug.

Comment 25

[georgi - hopefully not receiving bugspam]

i *suspect* this bug has something to do with events and modality

Comment 26

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #24)

But (unfortunately) the hang does prevent the attachment 215647 [details]
testcase from working on OS X (on the trunk and 1.9.0 branch).

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment #215647 [details] doesn't crash for me on an x86-32 trunk Linux debug build. Karl, can you have a go?

Comment 28

[Karl Tomlinson (:karlt)]

Comment on attachment 215647 [details]
testcase

I can't close the file dialog while the alert is showing (even though it appears to take focus).
Closing the file dialog after closing the alert doesn't cause a crash for me on mozilla-central or cvs trunk.

gtk+-2.12.9.  x86_64

Comment 29

[Karl Tomlinson (:karlt)]

Comment on attachment 305510 [details]
xul12-crash.xul - try typing in the input box

Bug 374011 prevents typing into the input box (even though it appears to still have focus after closing the file dialog).
Trying to type and quiting produced no crash for me on mozilla-central or cvs.

What gtk modules are being used (themes, extensions, IME)?  

lsof -p <firefox-bin-process> may give useful information.

Comment 30

[georgi - hopefully not receiving bugspam]

no extensions, no themes.

can't type in the file input but when i select a file with the mouse i crash on trunk and 3.0-latest immediately after selecting the file.

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, I crash on my Linux build if I wait for the alert to come up, dismiss it, then select a file. Good.

Comment 32

[georgi - hopefully not receiving bugspam]

>OK, I crash on my Linux build if I wait for the alert to come up, dismiss it,
>then select a file. Good.

this is step 4 in comment #21

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sorry, I missed that.

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes it. The "if (!mTextFrame)" check is useless since the nsFileControlFrame may have been deleted and the memory filled with nonzero garbage. Instead, move the filepicker code to the event listener, which won't die.

Comment 35

[georgi - hopefully not receiving bugspam]

is the file dialog the only problem in this bug?

synthetic events after blowing other controls?

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 331082 [details] [diff] [review]
fix

Should the patch contain some changes to .h file too?

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Good catch. It actually builds without the .h change...

Comment 38

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 331206 [details] [diff] [review]
fix v2

>-  if (!mTextFrame) {
>-    // We got destroyed while the filepicker was up.  Don't do anything here.
>+  if (!mFrame) {
>+    // The frame got destroyed while the filepicker was up.  Don't do
>+    // anything here.
>+    // (This listener itself can't be destroyed because the event listener
>+    // manager holds a strong reference to us while it fires the event.)
I was mainly thinking about this change, but the patch (bug 230998) which added !mTextFrame
doesn't make sense to me. And mFrame should be null if the frame is destroyed.
So r=me.
I'm not a sreviewer ;)

Comment 39

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 331206 [details] [diff] [review]
fix v2

hurry up and become a super-reviewer, Olli!

Comment 40

[Mats Palmgren (inactive)]

Comment on attachment 331206 [details] [diff] [review]
fix v2

>+nsFileControlFrame::MouseListener::MouseClick(nsIDOMEvent* aMouseEvent)
> {
>+  if (!mFrame)
>+    return NS_OK;

Can that happen?  mFrame is set to null only in Destroy()
after the listener has been unregistered. An assertion
would do here I think.

sr=mats, either way

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK I'll use an assertion there.

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed cbce63f35fbd

Comment 43

[Carsten Book [:Tomcat]]

Attachment: crash stack

fyi i crashed last night with a similar stack on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2pre) Gecko/2008082509 Firefox/3.0.2pre during the try to upload some screenshots to imageshack

Comment 44

[Samuel Sidler (old account; do not CC)]

roc: Can you create a 1.9.0 patch for this?

Comment 45

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: 1.9.0 patch

The patch applies cleanly to 1.9.0 so it should work. However, I can't verify that right now --- it builds on Mac, but as mentioned above the Mac build hangs before it gets to the part where you can crash. So it'd be helpful if someone else could apply the patch on Linux 1.9.0 and verify that the crash is fixed.

Comment 46

[Karl Tomlinson (:karlt)]

Comment on attachment 353613 [details] [diff] [review]
1.9.0 patch

Fixes the crash on attachment 215647 [details] for me with 1.9.0 on x86_64/Linux (and I was able to reproduce without the patch).

Comment 47

[Daniel Veditz [:dveditz]]

What does the "Depends on 389931" mean? I can't find a comment in either bug explaining the link. bug 389931 claimed to be a regression from a different bug, was it ultimately determined to be this bug? Does the patch in this bug require that patch too?

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm not sure but 389931 is already fixed in 1.9.0 so there's nothing to worry about.

Comment 49

[Daniel Veditz [:dveditz]]

Comment on attachment 353613 [details] [diff] [review]
1.9.0 patch

>+nsFileControlFrame::MouseListener::MouseClick(nsIDOMEvent* aMouseEvent)
> {
>+  NS_ASSERTION(mFrame, "We should have been unregistered");

Why an assert on the branch version? In the 1.9.1/trunk version you check for null and return in optimized code.

Comment 50

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See comment #40. The 1.9.0 patch is based on changeset cbce63f35fbd, which used the NS_ASSERTION.

Comment 51

[Daniel Veditz [:dveditz]]

Comment on attachment 353613 [details] [diff] [review]
1.9.0 patch

Approved for 1.9.0.6, a=dveditz for release-drivers.

Comment 52

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into 1.9.0.6

Comment 53

[Alexander Sack]

Cannot reproduce on 1.8.1 (actually cannot reproduce anywhere on linux), but since the MouseClick code is there i guess its good to have this.

Not on 1.8.0 as it seems though.

Comment 54

[Alexander Sack]

Attachment: 1.8.1 backport

please take a quick look if thats ok for 1.8.1

Comment 55

[Alexander Sack]

Comment on attachment 355947 [details] [diff] [review]
1.8.1 backport

please approve for 1.8.1 landing.

Comment 56

[Al Billings [:abillings - ex-MoCo]]

I just tried to verify this for 1.9.0.6. The attached testcase just hung my 3.0.6pre build the same way it did 3.0.5.

I used Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009011304 GranParadiso/3.0.6pre.

This doesn't seem to be fixed.

Comment 57

[Al Billings [:abillings - ex-MoCo]]

On Windows XP and Ubuntu 8.04, the original testcase (or the steps in comment 21) doesn't crash Firefox 3.0.5 either.

Comment 58

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The Mac hang is fixed by bug 436473 which hasn't landed on 1.9.0 yet.

Windows and Linux should crash in 3.0.5. Not sure why they wouldn't.

Comment 59

[Al Billings [:abillings - ex-MoCo]]

I've tried it again on Ubuntu 8.04 with a clean profile using a downloaded 3.0.5 (not the pre-installed one) and it still doesn't crash on Linux with the testcase.

So far, I cannot verify the original problem so I cannot verify the fix.

Comment 60

[Samuel Sidler (old account; do not CC)]

Roc: Can you help verify here? If they aren't crashing 3.0.5, is there some other way we can test (maybe debug build)?

Comment 61

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Not without debugging it in a VM, which seems like more work than it's worth.

Comment 62

[Karl Tomlinson (:karlt)]

I couldn't reproduce a crash with 3.0 or 3.0.5 x86/Linux release builds, nor
see anything bad from valgrind --tool=memcheck with 3.0.5.

For a debug build on x86_64/Linux:

  cvs up -r FIREFOX_3_0_5_RELEASE client.mk
  make -f client.mk checkout
  make -f client.mk build

    crash on steps in comment 21.

  cvs up -A client.mk
  make -f client.mk checkout (checkout start: Fri Jan 16 14:27:50 NZDT 2009)
  make -f client.mk build

    no crash on steps in comment 21.

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 355947 [details] [diff] [review]
1.8.1 backport

Approved for 1.8.1.21, a=dveditz for release-drivers.

Comment 64

[Alexander Sack]

Committed attachment 355947 [details] [diff] [review] to MOZILLA_1_8_BRANCH for 1.8.1.21

Checking in layout/forms/nsFileControlFrame.cpp;
/cvsroot/mozilla/layout/forms/nsFileControlFrame.cpp,v  <--  nsFileControlFrame.cpp
new revision: 3.172.12.7; previous revision: 3.172.12.6
done
Checking in layout/forms/nsFileControlFrame.h;
/cvsroot/mozilla/layout/forms/nsFileControlFrame.h,v  <--  nsFileControlFrame.h
new revision: 1.75.10.5; previous revision: 1.75.10.4
done

Comment 65

[seshu.parvataneni]

Looks like I am seeing a similar issue mentione in comment 21 with SeaMonkey 
version 2.0b2. But then again it does not happen with the older release versions and installations on my other computers running the same OS. I've tried a clean install (even deleting all the user profiles) but that wouldn't help. Let me know if you need any information related to the crash.

Comment 66

[seshu.parvataneni]

Hi, Can someone tell me how to reopen this bug. Too bad to see that I am having a similar problem with version 2.0 seamonkey rc1. This bug has been blocking me from using any page with the form type input. Thanks.

Comment 67

[Karl Tomlinson (:karlt)]

parvata:  It would be best to file a new bug.  It's possible that the cause of your issue is different from what was fixed here.