Closed Bug 331088 Opened 14 years ago Closed 11 years ago

crash with <input type=file> and events


(Core :: Layout, defect, P4, critical)






(Reporter: guninski, Assigned: roc)



(4 keywords, Whiteboard: [sg:critical?])


(8 files, 1 obsolete file)

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.
Attached file testcase
No problem on windows, because of bug 100180.
Severity: normal → critical
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
macosx seems safe
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?
Keywords: crash
Whiteboard: [sg:investigate wfm?]
Attached file 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
a simpler testcase still crashes with scary stack:

#5  <signal handler called>
#6  0xb73bbae7 in strstr () from /lib/i686/
#7  0xb7337e12 in XRenderFindDisplay () from /usr/lib/
#8  0xb733482a in XRenderFreeGlyphs () from /usr/lib/
#9  0xb77bc1e0 in cairo_xlib_surface_get_display () from /usr/lib/
#10 0xb779b2a7 in cairo_scaled_font_get_font_matrix ()
   from /usr/lib/
#11 0xb77907ef in cairo_create () from /usr/lib/
#12 0xb779095f in cairo_create () from /usr/lib/
#13 0xb779b99b in cairo_scaled_font_get_font_matrix ()
   from /usr/lib/
#14 0xb779bce4 in cairo_scaled_font_destroy () from /usr/lib/
#15 0xb7791c61 in cairo_font_face_status () from /usr/lib/
#16 0xb779185d in cairo_debug_reset_static_data () from /usr/lib/
#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/
(gdb) x/i $pc
0xb73bbae7 <strstr+39>: movzbl (%edx),%eax
(gdb) p/x $edx
$1 = 0x5a5a5a5a
This also seems to crash on windows with the patch from Mats.
Depends on: 389931
Flags: blocking1.9?
Keywords: testcase
btw, sometimes the crash is delayed or on exit
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Flags: wanted1.9.0.x+
Flags: blocking1.9-
Hmm, it doesn't seem to crash in a regular build, only in a debug build, for some reason.
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.
Whiteboard: [sg:investigate wfm?] → [sg:critical?]
Flags: tracking1.9+
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
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?
Attached file 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.
Attachment #328069 - Attachment mime type: application/octet-stream → text/plain
i don't seem to crash on linux trunk. such modal dialogs/events may be related to Bug 441505
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.
Attachment 305510 [details] does not do anything for me on Mac trunk debug. Is it still relevant?
at the time of filing this, as in description:
>top of stack is NS_RELEASE and events are involved in the stack
Which attachment are you talking about? Crashes around NS_RELEASE are bad, but attachment #215647 [details] isn't a crash (anymore?).
at the time of filing Attachment 215647 [details] crashed as in the description.
 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 (
    at /opt/joro/firefox-central/src/layout/forms/nsTextControlFrame.h:211
#6  0x000000000092107b in nsFileControlFrame::MouseClick (this=0x2aaaaec0d3e8, 
    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>, 
    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,

i mean it crashes on today trunk from central
the instructions in comment #21 crash 3.0-latest with same stack
(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.
i *suspect* this bug has something to do with events and modality
(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).
Attachment #215647 [details] doesn't crash for me on an x86-32 trunk Linux debug build. Karl, can you have a go?
Comment on attachment 215647 [details]

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 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.
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.
OK, I crash on my Linux build if I wait for the alert to come up, dismiss it, then select a file. Good.
>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
Attached patch fix (obsolete) — Splinter Review
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.
Assignee: nobody → roc
Attachment #331082 - Flags: superreview?
Attachment #331082 - Flags: review?
Attachment #331082 - Flags: superreview?(Olli.Pettay)
Attachment #331082 - Flags: superreview?
Attachment #331082 - Flags: review?(Olli.Pettay)
Attachment #331082 - Flags: review?
is the file dialog the only problem in this bug?

synthetic events after blowing other controls?
Comment on attachment 331082 [details] [diff] [review]

Should the patch contain some changes to .h file too?
Attached patch fix v2Splinter Review
Good catch. It actually builds without the .h change...
Attachment #331082 - Attachment is obsolete: true
Attachment #331206 - Flags: superreview?(Olli.Pettay)
Attachment #331206 - Flags: review?(Olli.Pettay)
Attachment #331082 - Flags: superreview?(Olli.Pettay)
Attachment #331082 - Flags: review?(Olli.Pettay)
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 ;)
Attachment #331206 - Flags: superreview?(Olli.Pettay)
Attachment #331206 - Flags: review?(Olli.Pettay)
Attachment #331206 - Flags: review+
Comment on attachment 331206 [details] [diff] [review]
fix v2

hurry up and become a super-reviewer, Olli!
Attachment #331206 - Flags: superreview?(mats.palmgren)
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
Attachment #331206 - Flags: superreview?(mats.palmgren) → superreview+
OK I'll use an assertion there.
Pushed cbce63f35fbd
Closed: 11 years ago
Resolution: --- → FIXED
Attached file 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: Gecko/2008082509 Firefox/3.0.2pre during the try to upload some screenshots to imageshack
Flags: blocking1.9.0.6?
roc: Can you create a 1.9.0 patch for this?
Attached patch 1.9.0 patchSplinter Review
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 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).
Flags: blocking1.9.0.6? → blocking1.9.0.6+
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?
I'm not sure but 389931 is already fixed in 1.9.0 so there's nothing to worry about.
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.
See comment #40. The 1.9.0 patch is based on changeset cbce63f35fbd, which used the NS_ASSERTION.
Comment on attachment 353613 [details] [diff] [review]
1.9.0 patch

Approved for, a=dveditz for release-drivers.
Attachment #353613 - Flags: approval1.9.0.6? → approval1.9.0.6+
Checked into
Keywords: fixed1.9.0.6
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.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x-
Attached patch 1.8.1 backportSplinter Review
please take a quick look if thats ok for 1.8.1
Attachment #355947 - Flags: review?(roc)
Attachment #355947 - Flags: superreview+
Attachment #355947 - Flags: review?(roc)
Attachment #355947 - Flags: review+
Comment on attachment 355947 [details] [diff] [review]
1.8.1 backport

please approve for 1.8.1 landing.
Attachment #355947 - Flags:
Flags: →
I just tried to verify this for 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: Gecko/2009011304 GranParadiso/3.0.6pre.

This doesn't seem to be fixed.
On Windows XP and Ubuntu 8.04, the original testcase (or the steps in comment 21) doesn't crash Firefox 3.0.5 either.
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.
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.
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)?
Not without debugging it in a VM, which seems like more work than it's worth.
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
  make -f checkout
  make -f build

    crash on steps in comment 21.

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

    no crash on steps in comment 21.
Comment on attachment 355947 [details] [diff] [review]
1.8.1 backport

Approved for, a=dveditz for release-drivers.
Attachment #355947 - Flags: →
Committed attachment 355947 [details] [diff] [review] to MOZILLA_1_8_BRANCH for

Checking in layout/forms/nsFileControlFrame.cpp;
/cvsroot/mozilla/layout/forms/nsFileControlFrame.cpp,v  <--  nsFileControlFrame.cpp
new revision:; previous revision:
Checking in layout/forms/nsFileControlFrame.h;
/cvsroot/mozilla/layout/forms/nsFileControlFrame.h,v  <--  nsFileControlFrame.h
new revision:; previous revision:
Keywords: fixed1.8.1.21
Group: core-security
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.