666529 - test_npn_timers.html bugs - crash and bad dependency on timer event ordering

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

###!!! ABORT: file /home/zoe/hg/mozilla-central/ipc/chromium/src/base/pickle.cc, line 98
Pickle::ReadBool(void**, bool*) const (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/pickle.cc:99)
mozilla::plugins::PPluginScriptableObjectParent::Read(mozilla::plugins::Variant*, IPC::Message const*, void**) (/home/zoe/hg/mozilla-central/new-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp:1459)
mozilla::plugins::PPluginScriptableObjectParent::Read(InfallibleTArray<mozilla::plugins::Variant>*, IPC::Message const*, void**) (/home/zoe/hg/mozilla-central/new-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp:1282)
mozilla::plugins::PPluginScriptableObjectParent::OnCallReceived(IPC::Message const&, IPC::Message*&) (/home/zoe/hg/mozilla-central/new-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp:757)
mozilla::plugins::PPluginModuleParent::OnCallReceived(IPC::Message const&, IPC::Message*&) (/home/zoe/hg/mozilla-central/new-firefox/ipc/ipdl/PPluginModuleParent.cpp:936)
mozilla::ipc::RPCChannel::DispatchIncall(IPC::Message const&) (/home/zoe/hg/mozilla-central/ipc/glue/RPCChannel.cpp:517)
mozilla::ipc::RPCChannel::OnMaybeDequeueOne() (/home/zoe/hg/mozilla-central/ipc/glue/RPCChannel.cpp:429)
MessageLoop::RunTask(Task*) (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:343)
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:353)
MessageLoop::DoWork() (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:450)
mozilla::ipc::DoWorkRunnable::Run() (/home/zoe/hg/mozilla-central/ipc/glue/MessagePump.cpp:71)
nsThread::ProcessNextEvent(int, int*) (/home/zoe/hg/mozilla-central/xpcom/threads/nsThread.cpp:618)
NS_ProcessNextEvent_P(nsIThread*, int) (/home/zoe/hg/mozilla-central/new-firefox/xpcom/build/nsThreadUtils.cpp:245)
mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/zoe/hg/mozilla-central/ipc/glue/MessagePump.cpp:111)
MessageLoop::RunInternal() (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:219)
~AutoRunState (/home/zoe/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:511)
nsBaseAppShell::Run() (/home/zoe/hg/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:191)
nsAppStartup::Run() (/home/zoe/hg/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:223)
XRE_main (/home/zoe/hg/mozilla-central/toolkit/xre/nsAppRunner.cpp:3567)
do_main (/home/zoe/hg/mozilla-central/browser/app/nsBrowserApp.cpp:199)
__libc_start_main (/build/buildd/eglibc-2.13/csu/libc-start.c:258)
_start (pthread_atfork.c:0)
###!!! ABORT: file /home/zoe/hg/mozilla-central/ipc/chromium/src/base/pickle.cc, line 98

###!!! [Child][RPCChannel] Error: Channel error: cannot send/recv

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

timerIdReceive is beyond the boundary of timerID array. It breaks timerTestResult value.

Comment 2

[Josh Aas]

I will land this today. Thanks for the fix!

Comment 3

[Josh Aas]

This fix actually causes the test to fail on my Mac OS X 10.6 machine. Without this fix the test passes. Something else seems to be wrong, looking into it.

Comment 4

[Josh Aas]

The reason the test fails is that timer events 5 and 6 (in the current code) should happen at the same time and their order isn't guaranteed. This test depends on a particular order for event firing - when the order is broken the test fails. I think we should change the events such that expected firings don't overlap.

Comment 5

[Josh Aas]

Attachment: fix v2.0

This solves the array length problem by only using two timers instead of expanding the array to three. It solves the unpredictable firing order problem by not having expected firings overlap.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #4)
> The reason the test fails is that timer events 5 and 6 (in the current code)
> should happen at the same time and their order isn't guaranteed. This test
> depends on a particular order for event firing - when the order is broken
> the test fails. 

And also one timer event is fired more than once? I was wondering why this test causes any crash on other platforms.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #6)
> (In reply to comment #4)
> > The reason the test fails is that timer events 5 and 6 (in the current code)
> > should happen at the same time and their order isn't guaranteed. This test
> > depends on a particular order for event firing - when the order is broken
> > the test fails. 
> 
> And also one timer event is fired more than once? I was wondering why this
> test causes any crash on other platforms.

oops!  I meant the test does not cause any crash..

Comment 8

[Jonathan Griffin (:jgriffin)]

Comment on attachment 541515 [details] [diff] [review]
fix v2.0

Looks good, thanks for fixing this!

Comment 9

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/8904812b90a7