Last Comment Bug 666529 - test_npn_timers.html bugs - crash and bad dependency on timer event ordering
: test_npn_timers.html bugs - crash and bad dependency on timer event ordering
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Josh Aas
:
:
Mentors:
Depends on:
Blocks: 658451
  Show dependency treegraph
 
Reported: 2011-06-23 02:38 PDT by Hiroyuki Ikezoe (:hiro)
Modified: 2011-06-24 19:14 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (904 bytes, patch)
2011-06-23 02:42 PDT, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
fix v2.0 (3.16 KB, patch)
2011-06-23 15:04 PDT, Josh Aas
jgriffin: review+
Details | Diff | Splinter Review

Description Hiroyuki Ikezoe (:hiro) 2011-06-23 02:38:00 PDT
###!!! 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) 2011-06-23 02:42:55 PDT
Created attachment 541321 [details] [diff] [review]
Fix

timerIdReceive is beyond the boundary of timerID array. It breaks timerTestResult value.
Comment 2 Josh Aas 2011-06-23 13:45:28 PDT
I will land this today. Thanks for the fix!
Comment 3 Josh Aas 2011-06-23 13:54:32 PDT
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 2011-06-23 14:58:52 PDT
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 2011-06-23 15:04:39 PDT
Created attachment 541515 [details] [diff] [review]
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) 2011-06-23 15:28:46 PDT
(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) 2011-06-23 15:29:54 PDT
(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) 2011-06-23 17:18:33 PDT
Comment on attachment 541515 [details] [diff] [review]
fix v2.0

Looks good, thanks for fixing this!
Comment 9 Josh Aas 2011-06-24 19:14:24 PDT
pushed to mozilla-central

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

Note You need to log in before you can comment on or make changes to this bug.