Skip to content

Commit d7f83aa

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 UltraBlame original commit: 12b53a3f19ff688bf80bd723e03511678147b7a7
1 parent 51e9158 commit d7f83aa

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

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)