1124170 - [Service Worker] Null pointer dereference in mozilla::dom::workers::UpdatePromise::AddPromise(mozilla::dom::Promise*)

Status: RESOLVED INVALID

(Core :: DOM: Workers, defect)

Description

[Julian Seward [:jseward]]

I am somewhat mystified by this, since I can only reproduce it when
running on Valgrind -- it happens every time.  I can't reproduce it
natively.

The immediate symptoms are as shown below.  In short, the call here
to AddPromise ..

dom/workers/ServiceWorkerManager.cpp:478
    // We append this register() call's promise after calling Update() because
    // we don't want this one to be aborted when the others (existing updates
    // for the same registration) are aborted. Update() sets a new
    // UpdatePromise on the registration.
    registration->mUpdatePromise->AddPromise(mPromise);

is being done on a NULL object.  That is,  registration->mUpdatePromise
is NULL.  The result is:


Invalid read of size 8
   at 0x78F0129: Length (/home/sewardj/MOZ/MC-CURR/ff-O2-linux64/dom/workers/../../dist/include/nsTArray.h:330)
   by 0x78F0129: AppendElement<mozilla::dom::Promise*&> (/home/sewardj/MOZ/MC-CURR/ff-O2-linux64/dom/workers/../../dist/include/nsTArray.h:1330)
   by 0x78F0129: AddPromise (/home/sewardj/MOZ/MC-CURR/dom/workers/ServiceWorkerManager.cpp:62)
   by 0x78F0129: mozilla::dom::workers::RegisterRunnable::Run() (/home/sewardj/MOZ/MC-CURR/dom/workers/ServiceWorkerManager.cpp:482)
   by 0x6086897: nsThread::ProcessNextEvent(bool, bool*) (/home/sewardj/MOZ/MC-CURR/xpcom/threads/nsThread.cpp:855)
   by 0x60A64DC: NS_ProcessNextEvent(nsIThread*, bool) (/home/sewardj/MOZ/MC-CURR/xpcom/glue/nsThreadUtils.cpp:265)
   by 0x634F3FA: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/sewardj/MOZ/MC-CURR/ipc/glue/MessagePump.cpp:99)
   by 0x632E341: RunInternal (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:233)
   by 0x632E341: RunHandler (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:226)
   by 0x632E341: MessageLoop::Run() (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:200)
   by 0x7A14F5D: nsBaseAppShell::Run() (/home/sewardj/MOZ/MC-CURR/widget/nsBaseAppShell.cpp:164)
   by 0x822DCFB: XRE_RunAppShell (/home/sewardj/MOZ/MC-CURR/toolkit/xre/nsEmbedFunctions.cpp:738)
   by 0x632E341: RunInternal (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:233)
   by 0x632E341: RunHandler (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:226)
   by 0x632E341: MessageLoop::Run() (/home/sewardj/MOZ/MC-CURR/ipc/chromium/src/base/message_loop.cc:200)
   by 0x822E37B: XRE_InitChildProcess (/home/sewardj/MOZ/MC-CURR/toolkit/xre/nsEmbedFunctions.cpp:575)
   by 0x403F8B: content_process_main(int, char**) [clone .part.0] (/home/sewardj/MOZ/MC-CURR/ipc/app/../contentproc/plugin-container.cpp:211)
   by 0xBA10FDF: (below main) (/usr/src/debug/glibc-2.20/csu/libc-start.c:289)
 Address 0x8 is not stack'd, malloc'd or (recently) free'd

Comment 1

[Julian Seward [:jseward]]

STR: build m-c with gcc-4.9 -Og on x86_64 linux (Fedora 21), then
run mochitest dom/workers/test/serviceworkers/test_get_serviced.html
thusly:

(DISPLAY=:1.0 G_SLICE=always-malloc MOZ_DISABLE_NONLOCAL_CONNECTIONS=0 \
  ./mach mochitest-plain --e10s \
  --debugger=/home/sewardj/VgTRUNK/mozhx/Inst/bin/valgrind \
  --debugger-args="--fair-sched=yes --smc-check=all-non-file --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no --trace-children=yes --child-silent-after-fork=yes --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java --num-transtab-sectors=24 --tool=memcheck --freelist-vol=500000000 --redzone-size=256 --gen-suppressions=no --vex-iropt-register-updates=allregs-at-mem-access --stats=yes --partial-loads-ok=yes --show-mismatched-frees=no --read-inline-info=yes --fullpath-after=-2015/ --num-callers=16 --track-origins=no" \
  dom/workers/test/serviceworkers/test_get_serviced.html) 2>&1 \
  | tee spew-29-mc

Comment 2

[Julian Seward [:jseward]]

More analysis #1

I added the following debug printing:

    fprintf(stderr, "XXXXXX1 registration = %p\n", registration.get());
    fprintf(stderr, "XXXXXX2 registration->mUpdatePromise = %p\n", registration->mUpdatePromise.get());
    registration->mUpdatePromise->AddPromise(mPromise);

and duly got

  TEST-START | /tests/dom/workers/test/serviceworkers/test_get_serviced.html
  XXXXXX1 registration = 0x2fcb5b10
  XXXXXX2 registration->mUpdatePromise = (nil)
  ==4653== Invalid read of size 8
  etc

Comment 3

[Julian Seward [:jseward]]

More analysis #2

I connected GDB to Valgrind's gdbserver, and poked around at the
point where the error is reported.  It doesn't tell anything much
useful -- merely confirming the crash stack and above diagnosis
re registration->mUpdatePromise being NULL.

###### the following at ServiceWorkerManager.cpp:62

(gdb) p aPromise
$1 = (mozilla::dom::Promise *) 0x2fca8630
(gdb) p this
$2 = (mozilla::dom::workers::UpdatePromise * const) 0x0
(gdb) up

###### the following at ServiceWorkerManager.cpp:482

#1  0x000000000773bce9 in mozilla::dom::workers::RegisterRunnable::Run (this=0x2fcaa920)
    at /home/sewardj/MOZ/MC-CURR/dom/workers/ServiceWorkerManager.cpp:482
482	    registration->mUpdatePromise->AddPromise(mPromise);
(gdb) p  registration->mUpdatePromise
Could not find operator->.
(gdb) p registration
$3 = {mRawPtr = 0x2fcb5bc0}
(gdb) p* registration
Could not find operator*.
(gdb) quit

Comment 4

[Julian Seward [:jseward]]

/me mystified, particularly on why it won't reproduce natively.
Any suggestions how to progress this?

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

UpdatePromise is gone courtesy bug 1113555