Worker: Fix broken GC logic on Dedicated Worker while DOMTimer is set

third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 14 matching lines @@
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "core/workers/InProcessWorkerObjectProxy.h"
 
 #include "bindings/core/v8/SerializedScriptValue.h"
 #include "bindings/core/v8/SourceLocation.h"
+#include "bindings/core/v8/V8GCController.h"
 #include "core/dom/CrossThreadTask.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExecutionContext.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/workers/InProcessWorkerMessagingProxy.h"
 #include "core/workers/ParentFrameTaskRunners.h"
+#include "core/workers/WorkerGlobalScope.h"
+#include "core/workers/WorkerThread.h"
 #include "platform/CrossThreadFunctional.h"
 #include "public/platform/WebTaskRunner.h"
 #include "wtf/Functional.h"
 #include "wtf/PtrUtil.h"
 #include <memory>
 
 namespace blink {
 
+const double kDefaultIntervalInSec = 1;
+const double kMaxIntervalInSec = 30;
+
 std::unique_ptr<InProcessWorkerObjectProxy> InProcessWorkerObjectProxy::create(InProcessWorkerMessagingProxy* messagingProxy)
 {
     DCHECK(messagingProxy);
     return wrapUnique(new InProcessWorkerObjectProxy(messagingProxy));
 }
 
+InProcessWorkerObjectProxy::~InProcessWorkerObjectProxy() {}
+
 void InProcessWorkerObjectProxy::postMessageToWorkerObject(PassRefPtr<SerializedScriptValue> message, std::unique_ptr<MessagePortChannelArray> channels)
 {
     getParentFrameTaskRunners()->get(TaskType::PostedMessage)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::postMessageToWorkerObject, crossThreadUnretained(m_messagingProxy), message, passed(std::move(channels))));
 }
 
 void InProcessWorkerObjectProxy::postTaskToMainExecutionContext(std::unique_ptr<ExecutionContextTask> task)
 {
     // TODO(hiroshige,yuryu): Make this not use ExecutionContextTask and use
     // getParentFrameTaskRunners() instead.
     getExecutionContext()->postTask(BLINK_FROM_HERE, std::move(task));
 }
 
-void InProcessWorkerObjectProxy::confirmMessageFromWorkerObject(bool hasPendingActivity)
+void InProcessWorkerObjectProxy::confirmMessageFromWorkerObject()
 {
-    getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::confirmMessageFromWorkerObject, crossThreadUnretained(m_messagingProxy), hasPendingActivity));
+    getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::confirmMessageFromWorkerObject, crossThreadUnretained(m_messagingProxy)));
 }
 
-void InProcessWorkerObjectProxy::reportPendingActivity(bool hasPendingActivity)
+void InProcessWorkerObjectProxy::startPendingActivityTimer()
 {
-    getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::reportPendingActivity, crossThreadUnretained(m_messagingProxy), hasPendingActivity));
+    if (m_timer->isActive()) {
+        // Avoid a timer flood, for example, caused by a burst of posted
+        // messages.
+        if (m_currentIntervalInSec == kDefaultIntervalInSec)

[haraken, 2016/08/17 06:30:47]

Does this branch really work? You're updating m_currentIntervalInSec to
m_currentIntervalInSec*1.5 at line 94. Hence, this branch wouldn't be helpful to
avoid the timer flood.

Given the overhead of creating cancellable tasks (which we're discussing in
another thread), I want to take a conservative approach here; i.e., if it's not
easy to avoid the timer flood, just return immediately when m_timer->isActive
returns true. It may cause a 30 sec of delay in the worst case, but it would be
better than causing the timer flood.

Maybe we could introduce another timer and do something like this:

void startPendingActivityTimer() {
  if (!m_anotherTimer.isActive())
    m_anotherTimer.startOneShot(1000);
}

void anotherTimerFired() {
  m_currentIntervalInSec = kDefaultIntervalInSec;
  m_timer->startOneShot(m_currentIntervalInSec);
  m_currentIntervalInSec = std::min(...);
}

[kinuko, 2016/08/17 22:50:10]

m_currentIntervalInSec is actually a next interval sec so it won't work.  We
could expose something similar to base::Timer::GetCurrentDelay() in Timer.h
(having that tends to be useful anyways) and check it to make this pattern
saner, but as I wrote I'm fine just returning immediately too.

(Separately-- reseting an ongoing timer feels relatively a common pattern, if
it's costly it's probably worth commenting that in Timer.h until we fix it)

[nhiroki, 2016/08/17 23:31:31]

Acknowledged.

[nhiroki, 2016/08/17 23:31:31]

Ooops... yes, this doesn't work.
Agreed. Changed this function to immediately return if isActive() returns true.

+            return;
+
+        // Restart the timer with the default interval duration to check newly
+        // initiated pending activities timely.
+        m_currentIntervalInSec = kDefaultIntervalInSec;
+    }
+
+    m_timer->startOneShot(m_currentIntervalInSec, BLINK_FROM_HERE);
+    m_currentIntervalInSec = std::min(m_currentIntervalInSec * 1.5, m_maxIntervalInSec);
 }
 
 void InProcessWorkerObjectProxy::reportException(const String& errorMessage, std::unique_ptr<SourceLocation> location, int exceptionId)
 {
     getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::reportException, crossThreadUnretained(m_messagingProxy), errorMessage, passed(location->clone()), exceptionId));
 }
 
 void InProcessWorkerObjectProxy::reportConsoleMessage(MessageSource source, MessageLevel level, const String& message, SourceLocation* location)
 {
     getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::reportConsoleMessage, crossThreadUnretained(m_messagingProxy), source, level, message, passed(location->clone())));
 }
 
 void InProcessWorkerObjectProxy::postMessageToPageInspector(const String& message)
 {
     ExecutionContext* context = getExecutionContext();
     if (context->isDocument()) {
         // TODO(hiroshige): consider using getParentFrameTaskRunners() here
         // too.
         toDocument(context)->postInspectorTask(BLINK_FROM_HERE, createCrossThreadTask(&InProcessWorkerMessagingProxy::postMessageToPageInspector, crossThreadUnretained(m_messagingProxy), message));
     }
 }
 
+void InProcessWorkerObjectProxy::didEvaluateWorkerScript(bool)
+{
+    startPendingActivityTimer();
+}
+
+void InProcessWorkerObjectProxy::workerGlobalScopeStarted(WorkerOrWorkletGlobalScope* globalScope)
+{
+    DCHECK(!m_workerGlobalScope);
+    m_workerGlobalScope = toWorkerGlobalScope(globalScope);
+    m_timer = wrapUnique(new Timer<InProcessWorkerObjectProxy>(this, &InProcessWorkerObjectProxy::checkPendingActivity));
+}
+
 void InProcessWorkerObjectProxy::workerGlobalScopeClosed()
 {
     getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::terminateWorkerGlobalScope, crossThreadUnretained(m_messagingProxy)));
 }
 
 void InProcessWorkerObjectProxy::workerThreadTerminated()
 {
     // This will terminate the MessagingProxy.
     getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::workerThreadTerminated, crossThreadUnretained(m_messagingProxy)));
 }
 
+void InProcessWorkerObjectProxy::willDestroyWorkerGlobalScope()
+{
+    m_timer.reset();
+    m_workerGlobalScope = nullptr;
+}
+
 InProcessWorkerObjectProxy::InProcessWorkerObjectProxy(InProcessWorkerMessagingProxy* messagingProxy)
     : m_messagingProxy(messagingProxy)
+    , m_currentIntervalInSec(kDefaultIntervalInSec)
+    , m_maxIntervalInSec(kMaxIntervalInSec)
 {
 }
 
 ParentFrameTaskRunners* InProcessWorkerObjectProxy::getParentFrameTaskRunners()
 {
     DCHECK(m_messagingProxy);
     return m_messagingProxy->getParentFrameTaskRunners();
 }
 
 ExecutionContext* InProcessWorkerObjectProxy::getExecutionContext()
 {
     DCHECK(m_messagingProxy);
     return m_messagingProxy->getExecutionContext();
 }
 
+void InProcessWorkerObjectProxy::checkPendingActivity(TimerBase*)
+{
+    bool hasPendingActivity = V8GCController::hasPendingActivity(m_workerGlobalScope->thread()->isolate(), m_workerGlobalScope);
+    if (!hasPendingActivity) {
+        // Report all activities are done.
+        getParentFrameTaskRunners()->get(TaskType::Internal)->postTask(BLINK_FROM_HERE, crossThreadBind(&InProcessWorkerMessagingProxy::pendingActivityFinished, crossThreadUnretained(m_messagingProxy)));
+
+        // Don't schedule a timer. It will be started again when a message event
+        // is dispatched.
+        m_currentIntervalInSec = kDefaultIntervalInSec;
+        return;
+    }
+
+    // There is still a pending activity. Check it later.
+    startPendingActivityTimer();
+}
+
 } // namespace blink