Skip to content
Permalink
Browse files
IPC stream work queues lack autoreleasepools
https://bugs.webkit.org/show_bug.cgi?id=240647
<rdar://problem/93575653>

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2022-05-23
Reviewed by Simon Fraser.

Add the autorelease pool to each iteration of IPC stream message
processing.

This prevents leaks where the callgraph ends up using Objective-C
APIs that instantiate autoreleased objects. From WebKit code,
examples are things like dictionary literals from IOSurface.mm and
strings. Similarly other platform APIs use other similar, typically
small temporary autoreleased objects. Compared to GPUP held graphics
objects, even long running GPUP processes did not accumulate that much
leaked memory. The footprint would hike up 1-6 mb per hour on repeated
animation in the simplest case.

* Source/WebKit/Platform/IPC/StreamConnectionWorkQueue.cpp:
(IPC::StreamConnectionWorkQueue::processStreams):
* Source/WebKit/Shared/IPCStreamTester.cpp:
(WebKit::releaseUseCountHolder):
(WebKit::IPCStreamTester::checkAutoreleasePool):
* Source/WebKit/Shared/IPCStreamTester.h:
* Source/WebKit/Shared/IPCStreamTester.messages.in:
* LayoutTests/ipc/stream-check-autoreleasepool-expected.txt: Added.
* LayoutTests/ipc/stream-check-autoreleasepool.html: Added.

Canonical link: https://commits.webkit.org/250854@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294628 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
kkinnunen-apple authored and webkit-commit-queue committed May 23, 2022
1 parent bfeb180 commit 91439ead53d9d9a94d3a883897cd9b086df81684
Showing 6 changed files with 91 additions and 0 deletions.
@@ -0,0 +1,3 @@

PASS Test that stream processing drains autorelease pool if needed for the platform

@@ -0,0 +1,42 @@
<!doctype html><!-- webkit-test-runner [ IPCTestingAPIEnabled=true ] -->
<title>Test that stream processing drains autorelease pool if needed for the platform</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<body>
<script>
promise_test(async t => {
if (!window.IPC)
return;
const defaultTimeout = 1000;
const bufferSize = 100;
const streamTesterID = 447;
for (const processTarget of IPC.processTargets) {
const streamConnection = IPC.createStreamClientConnection(processTarget, bufferSize);
IPC.sendMessage(processTarget, 0, IPC.messages.IPCTester_CreateStreamTester.name, [
{ type: 'uint64_t', value: streamTesterID },
{ type: 'StreamConnectionBuffer', value: streamConnection.streamBuffer() },
]);
const arguments = IPC.waitForMessage(processTarget, streamTesterID, IPC.messages.IPCStreamTesterProxy_WasCreated.name, defaultTimeout);
streamConnection.setSemaphores(arguments[0].value, arguments[1].value);

// Test starts here.
try {
let result = streamConnection.sendSyncMessage(streamTesterID, IPC.messages.IPCStreamTester_CheckAutoreleasePool.name, defaultTimeout, []);
let previousValue = result.arguments[0];

assert_equals(previousValue.type, "int32_t", `for ${ processTarget }`);
assert_equals(previousValue.value, 1, `for ${ processTarget }`);
// Autoreleasepool drains between message processing. Add an idle wait so that it no
// messages are posted, so the pool drains.
await new Promise((resolve) => setTimeout(resolve, 300));
result = streamConnection.sendSyncMessage(streamTesterID, IPC.messages.IPCStreamTester_CheckAutoreleasePool.name, defaultTimeout, []);
previousValue = result.arguments[0];
assert_equals(previousValue.type, "int32_t", `for ${ processTarget }`);
assert_equals(previousValue.value, 1, `for ${ processTarget }`);
} finally {
IPC.sendSyncMessage(processTarget, 0, IPC.messages.IPCTester_ReleaseStreamTester.name, defaultTimeout, [{ type: 'uint64_t', value: streamTesterID }]);
}
}
});
</script>
</body>
@@ -26,6 +26,10 @@
#include "config.h"
#include "StreamConnectionWorkQueue.h"

#if USE(FOUNDATION)
#include <wtf/AutodrainedPool.h>
#endif

namespace IPC {

StreamConnectionWorkQueue::StreamConnectionWorkQueue(const char* name)
@@ -120,6 +124,9 @@ void StreamConnectionWorkQueue::processStreams()
constexpr size_t defaultMessageLimit = 1000;
bool hasMoreToProcess = false;
do {
#if USE(FOUNDATION)
AutodrainedPool perProcessingIterationPool;
#endif
Deque<WTF::Function<void()>> functions;
Vector<Ref<StreamServerConnection>> connections;
{
@@ -34,6 +34,10 @@
#include "StreamConnectionWorkQueue.h"
#include "StreamServerConnection.h"

#if USE(FOUNDATION)
#include <CoreFoundation/CoreFoundation.h>
#endif

namespace WebKit {

RefPtr<IPCStreamTester> IPCStreamTester::create(IPC::Connection& connection, IPCStreamTesterIdentifier identifier, IPC::StreamConnectionBuffer&& stream)
@@ -101,6 +105,36 @@ void IPCStreamTester::syncCrashOnZero(int32_t value, CompletionHandler<void(int3
completionHandler(value);
}

#if USE(FOUNDATION)

namespace {
struct UseCountHolder {
std::shared_ptr<bool> value;
};
}

static void releaseUseCountHolder(CFAllocatorRef, const void* value)
{
delete static_cast<const UseCountHolder*>(value);
}

#endif

void IPCStreamTester::checkAutoreleasePool(CompletionHandler<void(int32_t)>&& completionHandler)
{
if (!m_autoreleasePoolCheckValue)
m_autoreleasePoolCheckValue = std::make_shared<bool>(true);
completionHandler(m_autoreleasePoolCheckValue.use_count());

#if USE(FOUNDATION)
static const CFArrayCallBacks arrayCallbacks {
.release = releaseUseCountHolder,
};
const void* values[] = { new UseCountHolder { m_autoreleasePoolCheckValue } };
CFArrayRef releaseDetector = CFArrayCreate(kCFAllocatorDefault, values, 1, &arrayCallbacks);
CFAutorelease(releaseDetector);
#endif
}
}

#endif
@@ -31,6 +31,7 @@
#include "ScopedActiveMessageReceiveQueue.h"
#include "SharedMemory.h"
#include "StreamMessageReceiver.h"
#include <memory>
#include <wtf/HashMap.h>

namespace IPC {
@@ -59,10 +60,12 @@ class IPCStreamTester final : public IPC::StreamMessageReceiver {
// Messages.
void syncMessageReturningSharedMemory1(uint32_t byteCount, CompletionHandler<void(SharedMemory::IPCHandle)>&&);
void syncCrashOnZero(int32_t, CompletionHandler<void(int32_t)>&&);
void checkAutoreleasePool(CompletionHandler<void(int32_t)>&&);

const Ref<IPC::StreamConnectionWorkQueue> m_workQueue;
const Ref<IPC::StreamServerConnection> m_streamConnection;
const IPCStreamTesterIdentifier m_identifier;
std::shared_ptr<bool> m_autoreleasePoolCheckValue;
};

}
@@ -25,6 +25,8 @@
messages -> IPCStreamTester NotRefCounted Stream {
SyncMessageReturningSharedMemory1(uint32_t byteCount) -> (WebKit::SharedMemory::IPCHandle handle) Synchronous NotStreamEncodableReply
SyncCrashOnZero(int32_t value) -> (int32_t sameValue) Synchronous

CheckAutoreleasePool() -> (int32_t previousUseCount) Synchronous
}

#endif

0 comments on commit 91439ea

Please sign in to comment.