Skip to content

Commit a301af9

Browse files
[2.38] limit conservative scan range
Turns out that conservative GC root scanning is going too deep into the stack and this may prevent the collection of objects - see: #1363 The change introduces 'stack guards' that prevent this in particular cases, that is - if there is gloop main loop routine in the stack the GC root stack search will not go deeper that this frame. This avoids some 'false positives' conservative roots that can make the app hold a lot of memory.
1 parent f0b98a4 commit a301af9

File tree

6 files changed

+82
-1
lines changed

6 files changed

+82
-1
lines changed

Source/JavaScriptCore/heap/MachineStackMarker.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <wtf/BitVector.h>
2828
#include <wtf/PageBlock.h>
2929
#include <wtf/StdLibExtras.h>
30+
#include <wtf/ConservativeScanStackGuards.h>
3031

3132
namespace JSC {
3233

@@ -44,7 +45,7 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot
4445
conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
4546
}
4647

47-
conservativeRoots.add(currentThreadState.stackTop, currentThreadState.stackOrigin, jitStubRoutines, codeBlocks);
48+
conservativeRoots.add(currentThreadState.stackTop, WTF::ConservativeScanStackGuards::updatedStackOrigin(currentThreadState.stackTop, currentThreadState.stackOrigin), jitStubRoutines, codeBlocks);
4849
}
4950

5051
static inline int osRedZoneAdjustment()

Source/WTF/wtf/CMakeLists.txt

+2
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ set(WTF_PUBLIC_HEADERS
4646
ConcurrentPtrHashSet.h
4747
ConcurrentVector.h
4848
Condition.h
49+
ConservativeScanStackGuards.h
4950
CountingLock.h
5051
CrossThreadCopier.h
5152
CrossThreadQueue.h
@@ -422,6 +423,7 @@ set(WTF_SOURCES
422423
CompilationThread.cpp
423424
ConcurrentBuffer.cpp
424425
ConcurrentPtrHashSet.cpp
426+
ConservativeScanStackGuards.cpp
425427
CountingLock.cpp
426428
CrossThreadCopier.cpp
427429
CrossThreadTaskHandler.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#include "ConservativeScanStackGuards.h"
2+
3+
namespace WTF {
4+
std::atomic_bool ConservativeScanStackGuards::active {true};
5+
thread_local std::deque<void*> ConservativeScanStackGuards::guard_ptr_stack;
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#pragma once
2+
3+
#include "Logging.h"
4+
5+
#include <atomic>
6+
#include <deque>
7+
8+
namespace WTF {
9+
class ConservativeScanStackGuards {
10+
public:
11+
struct ConservativeScanStackGuard {
12+
ConservativeScanStackGuard() {
13+
ConservativeScanStackGuards::setStackGuard(this);
14+
}
15+
~ConservativeScanStackGuard() {
16+
ConservativeScanStackGuards::resetStackGuard(this);
17+
}
18+
19+
private:
20+
// Prevent heap allocation
21+
void *operator new (size_t);
22+
void *operator new[] (size_t);
23+
void operator delete (void *);
24+
void operator delete[] (void*);
25+
};
26+
27+
friend struct ConservativeScanStackGuard;
28+
29+
static void* updatedStackOrigin(void *stackTop, void *stackOrigin) {
30+
if (!active.load() || guard_ptr_stack.empty()) {
31+
return stackOrigin;
32+
}
33+
34+
void *ret = stackOrigin;
35+
36+
void *guard_ptr = guard_ptr_stack.back();
37+
38+
if (guard_ptr > stackTop && guard_ptr < stackOrigin) {
39+
WTFLogAlways("ConservativeScanStackGuards: guard IN RANGE: stackTop: %p guard: %p stackOrigin: %p; correcting stackOrigin\n", stackTop, guard_ptr, stackOrigin);
40+
ret = guard_ptr;
41+
}
42+
return ret;
43+
}
44+
45+
static void setActive(bool act) {
46+
active.store(act);
47+
}
48+
49+
private:
50+
51+
static thread_local std::deque<void*> guard_ptr_stack;
52+
static std::atomic_bool active;
53+
54+
static void setStackGuard(void *ptr) {
55+
guard_ptr_stack.push_back(ptr);
56+
}
57+
58+
static void resetStackGuard(void *ptr) {
59+
if (ptr != guard_ptr_stack.back()) {
60+
WTFLogAlways("ConservativeScanStackGuards::resetStackGuard expected %p, had: %p", guard_ptr_stack.back(), ptr);
61+
}
62+
guard_ptr_stack.pop_back();
63+
}
64+
};
65+
}

Source/WTF/wtf/glib/RunLoopGLib.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
#include <wtf/MainThread.h>
3232
#include <wtf/glib/RunLoopSourcePriority.h>
3333

34+
#include "wtf/ConservativeScanStackGuards.h"
35+
3436
namespace WTF {
3537

3638
typedef struct {
@@ -103,6 +105,7 @@ void RunLoop::run()
103105
ASSERT(!runLoop.m_mainLoops.isEmpty());
104106

105107
GMainLoop* innermostLoop = runLoop.m_mainLoops[0].get();
108+
ConservativeScanStackGuards::ConservativeScanStackGuard guard;
106109
if (!g_main_loop_is_running(innermostLoop)) {
107110
g_main_context_push_thread_default(mainContext);
108111
g_main_loop_run(innermostLoop);

Source/WebKit/WebProcess/WebProcess.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,10 @@ void WebProcess::isJITEnabled(CompletionHandler<void(bool)>&& completionHandler)
10421042

10431043
void WebProcess::garbageCollectJavaScriptObjects()
10441044
{
1045+
{
1046+
JSLockHolder lock(commonVM());
1047+
commonVM().shrinkFootprintWhenIdle();
1048+
}
10451049
GCController::singleton().garbageCollectNow();
10461050
}
10471051

0 commit comments

Comments
 (0)