Title: [247094] trunk/Source/WebKit
- Revision
- 247094
- Author
- [email protected]
- Date
- 2019-07-03 10:48:41 -0700 (Wed, 03 Jul 2019)
Log Message
Crash under WTF::RefCounted<WebKit::TaskCounter>::deref()
https://bugs.webkit.org/show_bug.cgi?id=199453
<rdar://problem/51991477>
Reviewed by Youenn Fablet.
The crash was caused by StorageManager::suspend() getting called on the main thread but calling
its completion handler on a background queue. The completion handler was capturing a TaskCounter
object which is RefCounted (not ThreadSafeRefCounted).
Address the issue by making sure StorageManager::suspend() calls its completion handler on the
main thread. Also get rid of TaskCounter and use a WTF::CallbackAggregator instead.
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::actualPrepareToSuspend):
(WebKit::TaskCounter::TaskCounter): Deleted.
(WebKit::TaskCounter::~TaskCounter): Deleted.
* NetworkProcess/WebStorage/StorageManager.cpp:
(WebKit::StorageManager::suspend):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (247093 => 247094)
--- trunk/Source/WebKit/ChangeLog 2019-07-03 17:40:38 UTC (rev 247093)
+++ trunk/Source/WebKit/ChangeLog 2019-07-03 17:48:41 UTC (rev 247094)
@@ -1,3 +1,25 @@
+2019-07-03 Chris Dumez <[email protected]>
+
+ Crash under WTF::RefCounted<WebKit::TaskCounter>::deref()
+ https://bugs.webkit.org/show_bug.cgi?id=199453
+ <rdar://problem/51991477>
+
+ Reviewed by Youenn Fablet.
+
+ The crash was caused by StorageManager::suspend() getting called on the main thread but calling
+ its completion handler on a background queue. The completion handler was capturing a TaskCounter
+ object which is RefCounted (not ThreadSafeRefCounted).
+
+ Address the issue by making sure StorageManager::suspend() calls its completion handler on the
+ main thread. Also get rid of TaskCounter and use a WTF::CallbackAggregator instead.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::actualPrepareToSuspend):
+ (WebKit::TaskCounter::TaskCounter): Deleted.
+ (WebKit::TaskCounter::~TaskCounter): Deleted.
+ * NetworkProcess/WebStorage/StorageManager.cpp:
+ (WebKit::StorageManager::suspend):
+
2019-07-03 Youenn Fablet <[email protected]>
Make sure to cross-thread copy in StorageManager when hopping back to the main thread
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (247093 => 247094)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-07-03 17:40:38 UTC (rev 247093)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-07-03 17:48:41 UTC (rev 247094)
@@ -2029,16 +2029,6 @@
platformProcessDidTransitionToBackground();
}
-// FIXME: We can remove this one by adapting RefCounter.
-class TaskCounter : public RefCounted<TaskCounter> {
-public:
- explicit TaskCounter(Function<void()>&& callback) : m_callback(WTFMove(callback)) { }
- ~TaskCounter() { m_callback(); };
-
-private:
- Function<void()> m_callback;
-};
-
void NetworkProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend shouldAcknowledgeWhenReadyToSuspend)
{
#if PLATFORM(IOS_FAMILY)
@@ -2047,30 +2037,30 @@
lowMemoryHandler(Critical::Yes);
- RefPtr<TaskCounter> delayedTaskCounter;
+ RefPtr<CallbackAggregator> callbackAggregator;
if (shouldAcknowledgeWhenReadyToSuspend == ShouldAcknowledgeWhenReadyToSuspend::Yes) {
- delayedTaskCounter = adoptRef(new TaskCounter([this] {
+ callbackAggregator = CallbackAggregator::create([this] {
RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::notifyProcessReadyToSuspend() Sending ProcessReadyToSuspend IPC message", this);
if (parentProcessConnection())
parentProcessConnection()->send(Messages::NetworkProcessProxy::ProcessReadyToSuspend(), 0);
- }));
+ });
}
- platformPrepareToSuspend([delayedTaskCounter] { });
- platformSyncAllCookies([delayedTaskCounter] { });
+ platformPrepareToSuspend([callbackAggregator] { });
+ platformSyncAllCookies([callbackAggregator] { });
for (auto& connection : m_webProcessConnections)
- connection->cleanupForSuspension([delayedTaskCounter] { });
+ connection->cleanupForSuspension([callbackAggregator] { });
#if ENABLE(SERVICE_WORKER)
for (auto& server : m_swServers.values()) {
ASSERT(m_swServers.get(server->sessionID()) == server.get());
- server->startSuspension([delayedTaskCounter] { });
+ server->startSuspension([callbackAggregator] { });
}
#endif
for (auto& session : m_networkSessions)
- session.value->storageManager().suspend([delayedTaskCounter] { });
+ session.value->storageManager().suspend([callbackAggregator] { });
}
void NetworkProcess::processWillSuspendImminently()
Modified: trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp (247093 => 247094)
--- trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp 2019-07-03 17:40:38 UTC (rev 247093)
+++ trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp 2019-07-03 17:48:41 UTC (rev 247094)
@@ -936,11 +936,14 @@
Locker<Lock> stateLocker(m_stateLock);
ASSERT(m_state != State::Suspended);
- completionHandler();
+ if (m_state != State::WillSuspend) {
+ RunLoop::main().dispatch(WTFMove(completionHandler));
+ return;
+ }
- if (m_state != State::WillSuspend)
- return;
m_state = State::Suspended;
+ RunLoop::main().dispatch(WTFMove(completionHandler));
+
while (m_state == State::Suspended)
m_stateChangeCondition.wait(m_stateLock);
ASSERT(m_state == State::Running);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
