New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616775 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

http/tests/inspector-enabled/dedicated-workers-list.html crashes on WebKit Linux Leak

Project Member Reported by jbroman@chromium.org, Jun 2 2016

Issue description

Begins in https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/20074, but persists through a reland of the obvious CL in the blame list. I can't reproduce this locally, and the stderr output isn't helpful.

Assigning to relevant people for triage.
 
Let's wait for https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/20079 to cycle - it contains the reland.
Here's a snippet of the stdio from that run:

08:34:34.904 15683 renderer crash, pid = None, error_line = #CRASHED - renderer
08:34:34.904 15683 killed pid 2909
08:34:34.907 15683 worker/0 http/tests/inspector-enabled/dedicated-workers-list.html crashed, (stderr lines):
08:34:34.907 15683   [2909:3158:0602/083434:1120612491:WARNING:crash_handler_host_linux.cc(295)] Could not translate tid - assuming crashing thread is thread group leader; syscall_supported=1
08:34:34.909 15617 [9910/40874] http/tests/inspector-enabled/dedicated-workers-list.html failed unexpectedly (renderer crashed)
08:34:34.907 15683 worker/0 killing primary driver
08:34:34.908 15683 worker/0 killing secondary driver
08:34:34.908 15683 worker/0 http/tests/inspector-enabled/dedicated-workers-list.html failed:
08:34:34.908 15683 worker/0  renderer crashed

given this I expect that run to ultimately fail
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 2 2016

Cc: nhiroki@chromium.org
Thanks for reporting - I haven't looked into details here, but https://codereview.chromium.org/2011763002 landed around the same time.
Stack:

STDERR: Received signal 11 SEGV_MAPERR 000000000000
STDERR: #0 0x7fdaacb99607 base::debug::(anonymous namespace)::StackDumpSignalHandler()
STDERR: #1 0x7fdaa62f3bb0 <unknown>
STDERR: #2 0x7fdaa86abf95 v8::Object::Get()
STDERR: #3 0x7fdaac80fc8b blink::V8DebuggerImpl::getCompiledScripts()
STDERR: #4 0x7fdaac802a30 blink::V8DebuggerAgentImpl::enable()
STDERR: #5 0x7fdaac854ddf blink::protocol::Debugger::DispatcherImpl::enable()
STDERR: #6 0x7fdaac854cdb blink::protocol::Debugger::DispatcherImpl::dispatch()
STDERR: #7 0x7fdaac7b3fac blink::protocol::UberDispatcher::dispatch()
STDERR: #8 0x7fdaa7adfe3c blink::InspectorSession::dispatchProtocolMessage()
STDERR: #9 0x7fdaa7b00b74 blink::WorkerInspectorController::dispatchMessageFromFrontend()
STDERR: #10 0x7fdaa7bebdc1 blink::WorkerThread::runDebuggerTaskOnWorkerThread()
STDERR: #11 0x7fdaa7becd23 _ZN3WTF21PartBoundFunctionImplILNS_22FunctionThreadAffinityE0ESt5tupleIJOPN5blink12WorkerThreadEONS_13PassedWrapperISt10unique_ptrINS_8FunctionIFvvELS1_0EEESt14default_deleteISB_EEEEEENS_15F\
unctionWrapperIMS4_FvSE_EEEJEEclEv
STDERR: #12 0x7fdaa7bebe32 blink::WorkerThread::runDebuggerTaskDontWaitOnWorkerThread()
STDERR: #13 0x7fdaa960b8a9 _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0EEEENS0_9BindStateINS0_15RunnableAdapterIPFvSt10unique_ptrIN5blink13WebTaskRunner4TaskESt14default_deleteIS9_EEEEESD_JNS0_13PassedWrapperISC_\
EEEEELb0EFvvEE3RunEPNS0_13BindStateBaseE
STDERR: #14 0x7fdaacb9ac46 base::debug::TaskAnnotator::RunTask()
STDERR: #15 0x7fdaa9603927 scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
STDERR: #16 0x7fdaa9602a49 scheduler::TaskQueueManager::DoWork()
STDERR: #17 0x7fdaa96044aa _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0ELm1ELm2EEEENS0_9BindStateINS0_15RunnableAdapterIMN9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEEEFvPS7_S8_bEJNS_7WeakPtrIS7_EES8_bEEELb1EF\
vvEE3RunEPNS0_13BindStateBaseE
STDERR: #18 0x7fdaacb9ac46 base::debug::TaskAnnotator::RunTask()
STDERR: #19 0x7fdaacbbb515 base::MessageLoop::RunTask()

Cc: -nhiroki@chromium.org dgozman@chromium.org
Owner: nhiroki@chromium.org
Status: Started (was: Assigned)
Thank you for reporting this and sharing the stack. Probably this is caused by my change. I'll investigate...
A possible fix is in review: https://codereview.chromium.org/2041753002/
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48b51b66bbd7193a56102c49dc2b6ebea67274dc

commit 48b51b66bbd7193a56102c49dc2b6ebea67274dc
Author: nhiroki <nhiroki@chromium.org>
Date: Thu Jun 09 11:03:39 2016

Worker: Protect a running debugger task from forcible worker termination

A running debugger task must be protected from forcible worker termination, but
in the current implementation, there is a corner case where the debugger task
can suddenly be killed as follows:

1) performDebuggerTaskOnWorkerThread() is called almost at the same time as
   terminateInternal() on the main thread. terminateInternal() acquires
   |m_threadStateMutex| before performDebuggerTaskOnWorkerThread().
2) terminateInternal() checks a flag |m_runningDebuggerTask| before
   performDebuggerTaskOnWorkerThread() sets it, schedules a force termination
   task, and releases the lock.
3) performDebuggerTaskOnWorkerThread() acquires the lock, sets the flag, and
   runs the debugger task.
4) The scheduled task is fired while the debugger task is running and attempts
   to terminate it. This may cause crashes.

To avoid the case, this CL makes a scheduled task check the flag before
terminating the worker execution. In addition, this ensures that postTask() and
appendDebuggerTask() don't post new tasks after terminateInternal() or
prepareForShutdownOnWorkerThread() is called.

BUG= 616775 

Review-Url: https://codereview.chromium.org/2041753002
Cr-Commit-Position: refs/heads/master@{#398837}

[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/InspectorTaskRunner.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/WorkerInspectorController.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/WorkerThreadDebugger.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/workers/WorkerThread.h

I think the fix on c#9 is not sufficient. I'm now making another patch:
https://codereview.chromium.org/2046483002/
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48b51b66bbd7193a56102c49dc2b6ebea67274dc

commit 48b51b66bbd7193a56102c49dc2b6ebea67274dc
Author: nhiroki <nhiroki@chromium.org>
Date: Thu Jun 09 11:03:39 2016

Worker: Protect a running debugger task from forcible worker termination

A running debugger task must be protected from forcible worker termination, but
in the current implementation, there is a corner case where the debugger task
can suddenly be killed as follows:

1) performDebuggerTaskOnWorkerThread() is called almost at the same time as
   terminateInternal() on the main thread. terminateInternal() acquires
   |m_threadStateMutex| before performDebuggerTaskOnWorkerThread().
2) terminateInternal() checks a flag |m_runningDebuggerTask| before
   performDebuggerTaskOnWorkerThread() sets it, schedules a force termination
   task, and releases the lock.
3) performDebuggerTaskOnWorkerThread() acquires the lock, sets the flag, and
   runs the debugger task.
4) The scheduled task is fired while the debugger task is running and attempts
   to terminate it. This may cause crashes.

To avoid the case, this CL makes a scheduled task check the flag before
terminating the worker execution. In addition, this ensures that postTask() and
appendDebuggerTask() don't post new tasks after terminateInternal() or
prepareForShutdownOnWorkerThread() is called.

BUG= 616775 

Review-Url: https://codereview.chromium.org/2041753002
Cr-Commit-Position: refs/heads/master@{#398837}

[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/InspectorTaskRunner.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/WorkerInspectorController.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/inspector/WorkerThreadDebugger.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/48b51b66bbd7193a56102c49dc2b6ebea67274dc/third_party/WebKit/Source/core/workers/WorkerThread.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39ce89cd7e60e2f20925eb332d9d9735185425f4

commit 39ce89cd7e60e2f20925eb332d9d9735185425f4
Author: nhiroki <nhiroki@chromium.org>
Date: Thu Jun 16 10:02:18 2016

Worker: Protect a running debugger task from forcible worker termination (2)

A running debugger task must be protected from forcible worker termination, but
in the current implementation, there is a corner case where the debugger task
can suddenly be killed as follows:

1) terminate() is called while a debugger task is running. This does not
   schedule a task to forcibly terminate the worker execution.
2) terminateAndWait() is called while a debugger task is still running. This
   attempts to overtake the scheduled (actually not scheduled) termination task
   and forcibly terminates the worker execution.
3) The running debugger task is interrupted and possibly crashes.

After this change, terminateAndWait() respects the running debugger task on the
step 2).

BUG= 616775 

Review-Url: https://codereview.chromium.org/2046483002
Cr-Commit-Position: refs/heads/master@{#400120}

[modify] https://crrev.com/39ce89cd7e60e2f20925eb332d9d9735185425f4/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/39ce89cd7e60e2f20925eb332d9d9735185425f4/third_party/WebKit/Source/core/workers/WorkerThread.h
[modify] https://crrev.com/39ce89cd7e60e2f20925eb332d9d9735185425f4/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69876da721cf98624ded28086bf7f1107057e047

commit 69876da721cf98624ded28086bf7f1107057e047
Author: grunell <grunell@chromium.org>
Date: Thu Jun 16 11:56:22 2016

Revert of Worker: Protect a running debugger task from forcible worker termination (2) (patchset #1 id:180001 of https://codereview.chromium.org/2046483002/ )

Reason for revert:
Two compositorworker tests have failed flakily after the CL this CL depends on landed:

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/24612

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/24613

Need to revert this first.

Original issue's description:
> Worker: Protect a running debugger task from forcible worker termination (2)
>
> A running debugger task must be protected from forcible worker termination, but
> in the current implementation, there is a corner case where the debugger task
> can suddenly be killed as follows:
>
> 1) terminate() is called while a debugger task is running. This does not
>    schedule a task to forcibly terminate the worker execution.
> 2) terminateAndWait() is called while a debugger task is still running. This
>    attempts to overtake the scheduled (actually not scheduled) termination task
>    and forcibly terminates the worker execution.
> 3) The running debugger task is interrupted and possibly crashes.
>
> After this change, terminateAndWait() respects the running debugger task on the
> step 2).
>
> BUG= 616775 
>
> Committed: https://crrev.com/39ce89cd7e60e2f20925eb332d9d9735185425f4
> Cr-Commit-Position: refs/heads/master@{#400120}

TBR=kinuko@chromium.org,nhiroki@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 616775 

Review-Url: https://codereview.chromium.org/2069413003
Cr-Commit-Position: refs/heads/master@{#400134}

[modify] https://crrev.com/69876da721cf98624ded28086bf7f1107057e047/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/69876da721cf98624ded28086bf7f1107057e047/third_party/WebKit/Source/core/workers/WorkerThread.h
[modify] https://crrev.com/69876da721cf98624ded28086bf7f1107057e047/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20d56e4845354062512ee3655e7cf2501400dc7d

commit 20d56e4845354062512ee3655e7cf2501400dc7d
Author: nhiroki <nhiroki@chromium.org>
Date: Fri Jun 17 08:35:59 2016

[Reland] Worker: Protect a running debugger task from forcible worker termination (2)

Note: The original CL was reverted because another CL that it depends on was
reverted: https://codereview.chromium.org/2046483002/

A running debugger task must be protected from forcible worker termination, but
in the current implementation, there is a corner case where the debugger task
can suddenly be killed as follows:

1) terminate() is called while a debugger task is running. This does not
   schedule a task to forcibly terminate the worker execution.
2) terminateAndWait() is called while a debugger task is still running. This
   attempts to overtake the scheduled (actually not scheduled) termination task
   and forcibly terminates the worker execution.
3) The running debugger task is interrupted and possibly crashes.

After this change, terminateAndWait() respects the running debugger task on the
step 2).

BUG= 616775 
TBR=kinuko@chromium.org

Review-Url: https://codereview.chromium.org/2074863002
Cr-Commit-Position: refs/heads/master@{#400400}

[modify] https://crrev.com/20d56e4845354062512ee3655e7cf2501400dc7d/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/20d56e4845354062512ee3655e7cf2501400dc7d/third_party/WebKit/Source/core/workers/WorkerThread.h
[modify] https://crrev.com/20d56e4845354062512ee3655e7cf2501400dc7d/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp

Labels: M-53
A promising fix was landed. I'll monitor the flakiness dashboard.
It looks like the fix has been working fine. Let's remove the suppression.
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/555f7044bd81b9440d87cda62964f870d4eb2792

commit 555f7044bd81b9440d87cda62964f870d4eb2792
Author: nhiroki <nhiroki@chromium.org>
Date: Fri Jun 17 13:03:35 2016

Remove dedicated-workers-list.html from LeakExpectations

This was fixed by https://codereview.chromium.org/2074863002/

BUG= 616775 
TBR=nhiroki@chromium.org
NOTRY=true

Review-Url: https://codereview.chromium.org/2076883003
Cr-Commit-Position: refs/heads/master@{#400419}

[modify] https://crrev.com/555f7044bd81b9440d87cda62964f870d4eb2792/third_party/WebKit/LayoutTests/LeakExpectations

Status: Fixed (was: Started)

Sign in to add a comment