Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 1158d63

Browse files
committed
Bug 1868629 - Don't attempt to recover from failure when creating GC markers for the first time r=sfink
The recovery behaviour was added so that attempting to enable parallel marking by setting a GC parameter didn't cause a hard failure if there was an allocation failure initializing the extra markers. However the change made it so we would ignore failing to allocate the first marker (required) on initialization. This would have been caught by the OOM test we have for creating contexts, but this uses the 'always fail' OOM injection mode. This means that an earlier unhandled OOM condtions can be masked by a later one that is handled. For peace of mind I changed the test to run in both single failure and permanent failure modes. Differential Revision: https://phabricator.services.mozilla.com/D195770
1 parent de69219 commit 1158d63

File tree

2 files changed

+24
-17
lines changed

2 files changed

+24
-17
lines changed

js/src/gc/GC.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,9 @@ bool GCRuntime::init(uint32_t maxbytes) {
833833
}
834834
#endif
835835

836-
initOrDisableParallelMarking();
836+
if (!updateMarkersVector()) {
837+
return false;
838+
}
837839

838840
{
839841
AutoLockGCBgAlloc lock(this);
@@ -1334,6 +1336,8 @@ void GCRuntime::assertNoMarkingWork() const {
13341336
bool GCRuntime::initOrDisableParallelMarking() {
13351337
// Attempt to initialize parallel marking state or disable it on failure.
13361338

1339+
MOZ_ASSERT(markers.length() != 0);
1340+
13371341
if (!updateMarkersVector()) {
13381342
parallelMarkingEnabled = false;
13391343
return false;

js/src/jsapi-tests/testOOM.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,22 +33,25 @@ END_TEST(testOOM)
3333

3434
const uint32_t maxAllocsPerTest = 100;
3535

36-
# define START_OOM_TEST(name) \
37-
testName = name; \
38-
printf("Test %s: started: ", testName); \
39-
for (oomAfter = 1; oomAfter < maxAllocsPerTest; ++oomAfter) { \
40-
js::oom::simulator.simulateFailureAfter( \
41-
js::oom::FailureSimulator::Kind::OOM, oomAfter, \
42-
js::THREAD_TYPE_MAIN, true)
43-
44-
# define END_OOM_TEST \
45-
if (!js::oom::HadSimulatedOOM()) { \
46-
printf("\nTest %s: finished with %" PRIu64 " allocations\n", testName, \
47-
oomAfter - 1); \
48-
break; \
49-
} \
50-
} \
51-
js::oom::simulator.reset(); \
36+
# define START_OOM_TEST(name) \
37+
testName = name; \
38+
for (bool always : {false, true}) { \
39+
const char* subTest = always ? "fail always" : "fail once"; \
40+
printf("Test %s (%s): started: ", testName, subTest); \
41+
for (oomAfter = 1; oomAfter < maxAllocsPerTest; ++oomAfter) { \
42+
js::oom::simulator.simulateFailureAfter( \
43+
js::oom::FailureSimulator::Kind::OOM, oomAfter, \
44+
js::THREAD_TYPE_MAIN, always)
45+
46+
# define END_OOM_TEST \
47+
if (!js::oom::HadSimulatedOOM()) { \
48+
printf("\nTest %s (%s): finished with %" PRIu64 " allocations\n", \
49+
testName, subTest, oomAfter - 1); \
50+
break; \
51+
} \
52+
} \
53+
} \
54+
js::oom::simulator.reset(); \
5255
CHECK(oomAfter != maxAllocsPerTest)
5356

5457
# define MARK_STAR printf("*");

0 commit comments

Comments
 (0)