1010784 - Assertion failure when reusing sync XHR in worker.

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Shian-Yow Wu [:swu]]

+++ This bug was initially created as a clone of Bug #1008126 +++

  When resuing sync XHR and calling open/send again in worker, we got assertion failure followed by SIGSEGV.  The XHR reuse test case in bug 1008126 can be used to reproduce this issue.

Log:

Assertion failure: !mProxy->mSyncLoopTarget, at /home/sywu/work/mozilla-central/dom/workers/XMLHttpRequest.cpp:1488
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x0195629A]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x00A3A6DA]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x009CF1B8]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x00CC54B6]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x00CA2780]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x00CA27F4]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x0166B809]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x0237095C]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x022F0E4D]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x022F10F4]
XRE_main+0x000000DD [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/libxul.so +0x022F136D]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/firefox +0x00003D4B]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/firefox +0x0000420C]
__libc_start_main+0x000000ED [/lib/x86_64-linux-gnu/libc.so.6 +0x0002176D]
UNKNOWN [/home/sywu/work/mozilla-central/obj-x86_64-debug/dist/bin/firefox +0x00003709]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff207f058 in (anonymous namespace)::SendRunnable::MainThreadRun (this=0x7fffb9743400)
    at /home/sywu/work/mozilla-central/dom/workers/XMLHttpRequest.cpp:1479
1479	      MOZ_ASSERT(false, "This should never fail!");
(gdb) bt
#0  0x00007ffff207f058 in (anonymous namespace)::SendRunnable::MainThreadRun (this=0x7fffb9743400)
    at /home/sywu/work/mozilla-central/dom/workers/XMLHttpRequest.cpp:1479
#1  0x00007ffff207c29a in (anonymous namespace)::WorkerThreadProxySyncRunnable::Run (this=0x7fffb9743400)
    at /home/sywu/work/mozilla-central/dom/workers/XMLHttpRequest.cpp:1368
#2  0x00007ffff11606da in ProcessNextEvent (result=0x7fffffffc2df, mayWait=true, this=0x7ffff6a641a0)
    at /home/sywu/work/mozilla-central/xpcom/threads/nsThread.cpp:715
#3  nsThread::ProcessNextEvent (this=0x7ffff6a641a0, mayWait=true, result=0x7fffffffc2df)
    at /home/sywu/work/mozilla-central/xpcom/threads/nsThread.cpp:639
#4  0x00007ffff10f51b8 in NS_ProcessNextEvent (thread=<optimized out>, mayWait=true)
    at /home/sywu/work/mozilla-central/xpcom/glue/nsThreadUtils.cpp:263
#5  0x00007ffff13eb4b6 in mozilla::ipc::MessagePump::Run (this=0x7fffe6b8fd00, aDelegate=0x7fffe6b6c840)
    at /home/sywu/work/mozilla-central/ipc/glue/MessagePump.cpp:136
#6  0x00007ffff13c8780 in MessageLoop::RunInternal (this=0x7fffe6b6c840)
    at /home/sywu/work/mozilla-central/ipc/chromium/src/base/message_loop.cc:229
#7  0x00007ffff13c87f4 in RunHandler (this=0x7fffe6b6c840)
    at /home/sywu/work/mozilla-central/ipc/chromium/src/base/message_loop.cc:222
#8  MessageLoop::Run (this=0x7fffe6b6c840)
    at /home/sywu/work/mozilla-central/ipc/chromium/src/base/message_loop.cc:196
#9  0x00007ffff1d91809 in nsBaseAppShell::Run (this=0x7fffdfa527f0)
    at /home/sywu/work/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:164
#10 0x00007ffff2a9695c in nsAppStartup::Run (this=0x7fffdfa32240)
    at /home/sywu/work/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:278
#11 0x00007ffff2a16e4d in XREMain::XRE_mainRun (this=0x7fffffffc740)
    at /home/sywu/work/mozilla-central/toolkit/xre/nsAppRunner.cpp:4023
#12 0x00007ffff2a170f4 in XREMain::XRE_main (this=0x7fffffffc740, argc=5, argv=<optimized out>, aAppData=
---Type <return> to continue, or q <return> to quit---
    0x7fffffffc8f0) at /home/sywu/work/mozilla-central/toolkit/xre/nsAppRunner.cpp:4092
#13 0x00007ffff2a1736d in XRE_main (argc=5, argv=0x7fffffffdc58, aAppData=0x7fffffffc8f0, aFlags=<optimized out>)
    at /home/sywu/work/mozilla-central/toolkit/xre/nsAppRunner.cpp:4304
#14 0x0000000000403d4b in do_main (argc=5, argv=0x7fffffffdc58, xreDirectory=0x7ffff6a2d3c0)
    at /home/sywu/work/mozilla-central/browser/app/nsBrowserApp.cpp:282
#15 0x000000000040420c in main (argc=5, argv=0x7fffffffdc58)
    at /home/sywu/work/mozilla-central/browser/app/nsBrowserApp.cpp:643
(gdb)

Comment 1

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

Could you upload a minimal testcase.

Comment 2

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

oops, I missed the comment about testcase.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Why is this security sensitive?  It's just a null deref, no?

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Either the variant is not writable (not sure if that's possible) or we're OOM here.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

In any case this isn't s-s since we're just sending empty data rather than the correct data.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

khuey points out that there are two assertion failures listed in comment 0. Not sure what's going on here.

Comment 7

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

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #3)
> Why is this security sensitive?  It's just a null deref, no?
Just because I'm not familiar with that code and thread handling related assertions looked suspicious.
But good if not sec-sensitive.

Comment 8

[Shian-Yow Wu [:swu]]

Attachment: Test case.

This is a minimal test case to reproduce.

Comment 9

[Shian-Yow Wu [:swu]]

Attachment: Patch: Set mSyncLoopTarget to nullptr after done sending.

Set mSyncLoopTarget to nullptr after done sending.

Could you review this patch?

Comment 10

[Shian-Yow Wu [:swu]]

(In reply to <vacation until may 27> from comment #6)
> khuey points out that there are two assertion failures listed in comment 0.
> Not sure what's going on here.

There was actually only one assertion failure at MOZ_ASSERT(!mProxy->mSyncLoopTarget).  The 2nd assertion failure shown in backtrace should be false information caused by compiler optimization.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8424710 [details] [diff] [review]
Patch: Set mSyncLoopTarget to nullptr after done sending.

Review of attachment 8424710 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

This turned out to be really helpful in fixing bug 965309 because it showed me where to look.  Thanks for the patch Shian-Yow.

https://hg.mozilla.org/integration/mozilla-inbound/rev/05ec9bfe2eaa

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/05ec9bfe2eaa

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch (as landed)

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8430903 [details] [diff] [review]
Patch (as landed)

[Approval Request Comment]
See https://bugzilla.mozilla.org/show_bug.cgi?id=965309#c42

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/30ac291a7ed6
https://hg.mozilla.org/releases/mozilla-beta/rev/f2a2af61e05d

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/f2a2af61e05d

Comment 17

[Shian-Yow Wu [:swu]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #11)
> Comment on attachment 8424710 [details] [diff] [review]
> Patch: Set mSyncLoopTarget to nullptr after done sending.
> 
> Review of attachment 8424710 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me
> 
> This turned out to be really helpful in fixing bug 965309 because it showed
> me where to look.  Thanks for the patch Shian-Yow.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/05ec9bfe2eaa

Thanks, good to know that!