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

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: hiro, Assigned: Josh Aas)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.16 KB, patch
jgriffin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
###!!! 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
(Reporter)

Comment 1

6 years ago
Created attachment 541321 [details] [diff] [review]
Fix

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

Updated

6 years ago
Attachment #541321 - Flags: review?(joshmoz)
(Assignee)

Updated

6 years ago
Assignee: nobody → hiikezoe
(Assignee)

Updated

6 years ago
Attachment #541321 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 2

6 years ago
I will land this today. Thanks for the fix!
(Assignee)

Updated

6 years ago
Blocks: 658451
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
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.
(Assignee)

Updated

6 years ago
Attachment #541321 - Flags: review+
(Assignee)

Comment 5

6 years ago
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.
Attachment #541321 - Attachment is obsolete: true
Attachment #541515 - Flags: review?(jgriffin)
(Reporter)

Comment 6

6 years ago
(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.
(Reporter)

Comment 7

6 years ago
(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 on attachment 541515 [details] [diff] [review]
fix v2.0

Looks good, thanks for fixing this!
Attachment #541515 - Flags: review?(jgriffin) → review+
(Assignee)

Updated

6 years ago
Assignee: hiikezoe → joshmoz
(Assignee)

Comment 9

6 years ago
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/8904812b90a7
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Summary: Crash dom/plugins/test/test_npn_timers.html → test_npn_timers.html bugs - crash and bad dependency on timer event ordering
You need to log in before you can comment on or make changes to this bug.