diff options
author | Andreas Kling <kling@serenityos.org> | 2021-09-11 16:44:40 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-09-11 16:52:03 +0200 |
commit | c364520c2402b4b395f4e43fcfe3d4af4d546810 (patch) | |
tree | 0de3e6d60caea72831f3807b473b20d1bb2acdf5 | |
parent | 57371f7608b0d3251be03192e03a70b8b36ea3f2 (diff) | |
download | serenity-c364520c2402b4b395f4e43fcfe3d4af4d546810.zip |
LibJS+js+test-js: Add GC debug mode that keeps cells "alive" as zombies
This patch adds a `-z` option to js and test-js. When run in this mode,
garbage cells are never actually destroyed. We instead keep them around
in a special zombie state.
This allows us to validate that zombies don't get marked in future GC
scans (since there were not supposed to be any more references!) :^)
Cells get notified when they become a zombie (via did_become_zombie())
and this is used by WeakContainer cells to deregister themselves from
the heap.
-rw-r--r-- | Userland/Libraries/LibJS/Heap/Cell.h | 3 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Heap/Heap.cpp | 14 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Heap/Heap.h | 3 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h | 1 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/WeakMap.h | 2 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/WeakRef.h | 1 | ||||
-rw-r--r-- | Userland/Libraries/LibJS/Runtime/WeakSet.h | 2 | ||||
-rw-r--r-- | Userland/Libraries/LibTest/JavaScriptTestRunner.h | 2 | ||||
-rw-r--r-- | Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp | 2 | ||||
-rw-r--r-- | Userland/Utilities/js.cpp | 4 |
10 files changed, 33 insertions, 1 deletions
diff --git a/Userland/Libraries/LibJS/Heap/Cell.h b/Userland/Libraries/LibJS/Heap/Cell.h index f2229c9e6e..6e3e107988 100644 --- a/Userland/Libraries/LibJS/Heap/Cell.h +++ b/Userland/Libraries/LibJS/Heap/Cell.h @@ -24,9 +24,12 @@ public: bool is_marked() const { return m_mark; } void set_marked(bool b) { m_mark = b; } + virtual void did_become_zombie() { } + enum class State { Live, Dead, + Zombie, }; State state() const { return m_state; } diff --git a/Userland/Libraries/LibJS/Heap/Heap.cpp b/Userland/Libraries/LibJS/Heap/Heap.cpp index 4c5b9292d0..4e7ccdf6fb 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.cpp +++ b/Userland/Libraries/LibJS/Heap/Heap.cpp @@ -183,6 +183,13 @@ public: if (cell.is_marked()) return; dbgln_if(HEAP_DEBUG, " ! {}", &cell); + + if (cell.state() == Cell::State::Zombie) { + dbgln("BUG! Marking a zombie cell, {} @ {:p}", cell.class_name(), &cell); + cell.vm().dump_backtrace(); + VERIFY_NOT_REACHED(); + } + cell.set_marked(true); cell.visit_edges(*this); } @@ -223,7 +230,12 @@ void Heap::sweep_dead_cells(bool print_report, const Core::ElapsedTimer& measure dbgln_if(HEAP_DEBUG, " ~ {}", cell); if (should_store_swept_cells) swept_cells.append(cell); - block.deallocate(cell); + if (m_zombify_dead_cells) { + cell->set_state(Cell::State::Zombie); + cell->did_become_zombie(); + } else { + block.deallocate(cell); + } ++collected_cells; collected_cell_bytes += block.cell_size(); } else { diff --git a/Userland/Libraries/LibJS/Heap/Heap.h b/Userland/Libraries/LibJS/Heap/Heap.h index 8f7ac0d39e..99cb29743c 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.h +++ b/Userland/Libraries/LibJS/Heap/Heap.h @@ -69,6 +69,8 @@ public: bool should_collect_on_every_allocation() const { return m_should_collect_on_every_allocation; } void set_should_collect_on_every_allocation(bool b) { m_should_collect_on_every_allocation = b; } + void set_zombify_dead_cells(bool b) { m_zombify_dead_cells = b; } + void did_create_handle(Badge<HandleImpl>, HandleImpl&); void did_destroy_handle(Badge<HandleImpl>, HandleImpl&); @@ -127,6 +129,7 @@ private: bool m_should_gc_when_deferral_ends { false }; bool m_collecting_garbage { false }; + bool m_zombify_dead_cells { false }; }; } diff --git a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h index 27ef6e1695..6affdcd6a3 100644 --- a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h +++ b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h @@ -34,6 +34,7 @@ public: private: virtual void visit_edges(Visitor& visitor) override; + virtual void did_become_zombie() override { deregister(); } FunctionObject* m_cleanup_callback { nullptr }; diff --git a/Userland/Libraries/LibJS/Runtime/WeakMap.h b/Userland/Libraries/LibJS/Runtime/WeakMap.h index 18a6b74305..2f7c01b2d1 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakMap.h +++ b/Userland/Libraries/LibJS/Runtime/WeakMap.h @@ -30,6 +30,8 @@ public: virtual void remove_swept_cells(Badge<Heap>, Span<Cell*>) override; private: + virtual void did_become_zombie() override { deregister(); } + HashMap<Cell*, Value> m_values; // This stores Cell pointers instead of Object pointers to aide with sweeping }; diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.h b/Userland/Libraries/LibJS/Runtime/WeakRef.h index d93202ed55..40a370105e 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.h +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.h @@ -31,6 +31,7 @@ public: private: virtual void visit_edges(Visitor&) override; + virtual void did_become_zombie() override { deregister(); } Object* m_value { nullptr }; u32 m_last_execution_generation { 0 }; diff --git a/Userland/Libraries/LibJS/Runtime/WeakSet.h b/Userland/Libraries/LibJS/Runtime/WeakSet.h index 36433f86c5..2206037c70 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakSet.h +++ b/Userland/Libraries/LibJS/Runtime/WeakSet.h @@ -30,6 +30,8 @@ public: virtual void remove_swept_cells(Badge<Heap>, Span<Cell*>) override; private: + virtual void did_become_zombie() override { deregister(); } + HashTable<Cell*> m_values; // This stores Cell pointers instead of Object pointers to aide with sweeping }; diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunner.h b/Userland/Libraries/LibTest/JavaScriptTestRunner.h index c10e05d945..801aa847f1 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunner.h +++ b/Userland/Libraries/LibTest/JavaScriptTestRunner.h @@ -111,6 +111,7 @@ static consteval size_t __testjs_last() { return (AK::Detail::IntegralConstant<s static constexpr auto TOP_LEVEL_TEST_NAME = "__$$TOP_LEVEL$$__"; extern RefPtr<JS::VM> g_vm; extern bool g_collect_on_every_allocation; +extern bool g_zombify_dead_cells; extern bool g_run_bytecode; extern bool g_dump_bytecode; extern String g_currently_running_test; @@ -266,6 +267,7 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path) JS::VM::InterpreterExecutionScope scope(*interpreter); interpreter->heap().set_should_collect_on_every_allocation(g_collect_on_every_allocation); + interpreter->heap().set_zombify_dead_cells(g_zombify_dead_cells); if (g_run_file) { auto result = g_run_file(test_path, *interpreter); diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp b/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp index 8bf878ad86..2a7355b41c 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp +++ b/Userland/Libraries/LibTest/JavaScriptTestRunnerMain.cpp @@ -18,6 +18,7 @@ namespace JS { RefPtr<::JS::VM> g_vm; bool g_collect_on_every_allocation = false; +bool g_zombify_dead_cells = false; bool g_run_bytecode = false; bool g_dump_bytecode = false; String g_currently_running_test; @@ -109,6 +110,7 @@ int main(int argc, char** argv) }); args_parser.add_option(print_json, "Show results as JSON", "json", 'j'); args_parser.add_option(g_collect_on_every_allocation, "Collect garbage after every allocation", "collect-often", 'g'); + args_parser.add_option(g_zombify_dead_cells, "Zombify dead cells (to catch missing GC marks)", "zombify-dead-cells", 'z'); args_parser.add_option(g_run_bytecode, "Use the bytecode interpreter", "run-bytecode", 'b'); args_parser.add_option(g_dump_bytecode, "Dump the bytecode", "dump-bytecode", 'd'); args_parser.add_option(test_glob, "Only run tests matching the given glob", "filter", 'f', "glob"); diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index 0e152b7e08..25dd94ef7b 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -1079,6 +1079,7 @@ public: int main(int argc, char** argv) { bool gc_on_every_allocation = false; + bool zombify_dead_cells = false; bool disable_syntax_highlight = false; Vector<String> script_paths; @@ -1091,6 +1092,7 @@ int main(int argc, char** argv) args_parser.add_option(s_as_module, "Treat as module", "as-module", 'm'); args_parser.add_option(s_print_last_result, "Print last result", "print-last-result", 'l'); args_parser.add_option(gc_on_every_allocation, "GC on every allocation", "gc-on-every-allocation", 'g'); + args_parser.add_option(zombify_dead_cells, "Zombify dead cells (to catch missing GC marks)", "zombify-dead-cells", 'z'); args_parser.add_option(disable_syntax_highlight, "Disable live syntax highlighting", "no-syntax-highlight", 's'); args_parser.add_positional_argument(script_paths, "Path to script files", "scripts", Core::ArgsParser::Required::No); args_parser.parse(argc, argv); @@ -1131,6 +1133,7 @@ int main(int argc, char** argv) ReplConsoleClient console_client(interpreter->global_object().console()); interpreter->global_object().console().set_client(console_client); interpreter->heap().set_should_collect_on_every_allocation(gc_on_every_allocation); + interpreter->heap().set_zombify_dead_cells(zombify_dead_cells); interpreter->vm().set_underscore_is_last_value(true); s_editor = Line::Editor::construct(); @@ -1334,6 +1337,7 @@ int main(int argc, char** argv) ReplConsoleClient console_client(interpreter->global_object().console()); interpreter->global_object().console().set_client(console_client); interpreter->heap().set_should_collect_on_every_allocation(gc_on_every_allocation); + interpreter->heap().set_zombify_dead_cells(zombify_dead_cells); signal(SIGINT, [](int) { sigint_handler(); |