diff --git a/fxjs/gc/heap.cpp b/fxjs/gc/heap.cpp index 7161cf4406..a73c0c5495 100644 --- a/fxjs/gc/heap.cpp +++ b/fxjs/gc/heap.cpp @@ -84,6 +84,10 @@ FXGCScopedHeap FXGC_CreateHeap() { return FXGCScopedHeap(heap.release()); } +std::shared_ptr FXGC_CreatePlatformForTesting() { + return std::make_shared(); +} + void FXGC_ForceGarbageCollection(cppgc::Heap* heap) { heap->ForceGarbageCollectionSlow("FXGC", "ForceGarbageCollection", cppgc::Heap::StackState::kNoHeapPointers); diff --git a/fxjs/gc/heap.h b/fxjs/gc/heap.h index 1e2bf61590..8877eb9dfb 100644 --- a/fxjs/gc/heap.h +++ b/fxjs/gc/heap.h @@ -24,10 +24,17 @@ struct FXGCHeapDeleter { using FXGCScopedHeap = std::unique_ptr; +namespace cppgc { +class Platform; +} // namespace cppgc + void FXGC_Initialize(v8::Platform* platform, v8::Isolate* isolate); void FXGC_Release(); FXGCScopedHeap FXGC_CreateHeap(); void FXGC_ForceGarbageCollection(cppgc::Heap* heap); +// Exposes the internal CFXGC_Platform so tests can verify the +// cppgc::Platform contract directly. +std::shared_ptr FXGC_CreatePlatformForTesting(); #define CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED \ template \ diff --git a/fxjs/gc/heap_unittest.cpp b/fxjs/gc/heap_unittest.cpp index 126dad6d37..8013aad20d 100644 --- a/fxjs/gc/heap_unittest.cpp +++ b/fxjs/gc/heap_unittest.cpp @@ -7,12 +7,15 @@ #include #include +#include "core/fxcrt/cfx_timer.h" #include "core/fxcrt/containers/contains.h" #include "testing/fxgc_unittest.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/v8_test_environment.h" #include "v8/include/cppgc/allocation.h" #include "v8/include/cppgc/persistent.h" +#include "v8/include/cppgc/platform.h" +#include "v8/include/v8-platform.h" namespace { @@ -173,3 +176,133 @@ TEST_F(HeapUnitTest, Bloat) { Pump(); // Do not force GC, must happen implicitly when space required. } } + +// --------------------------------------------------------------------------- +// POC: CFXGC_Platform override mismatch leaves cppgc Sweeper without a +// foreground task runner, so finalizers for dead objects are never scheduled. +// --------------------------------------------------------------------------- + +namespace { + +// Mimics CFWL_Caret's pattern: a finalizable Oilpan object that owns a +// CFX_Timer registered in the global g_pwl_timer_map. The timer's +// |callback_iface_| is a raw back-reference into this Oilpan object that is +// only cleared when this object's *destructor* runs (not its prefinalizer). +class GCedTimerOwner final : public cppgc::GarbageCollected, + public CFX_Timer::CallbackIface { + public: + static int s_live; + static int s_fired; + + explicit GCedTimerOwner(CFX_Timer::HandlerIface* handler) { + ++s_live; + timer_ = std::make_unique(handler, this, 100); + } + ~GCedTimerOwner() override { --s_live; } + + void OnTimerFired() override { ++s_fired; } + void Trace(cppgc::Visitor* visitor) const {} + + private: + std::unique_ptr timer_; +}; + +int GCedTimerOwner::s_live = 0; +int GCedTimerOwner::s_fired = 0; + +class FakeTimerHandler final : public CFX_Timer::HandlerIface { + public: + int32_t SetTimer(int32_t uElapse, TimerCallback lpTimerFunc) override { + fn_ = lpTimerFunc; + return ++next_id_; + } + void KillTimer(int32_t nTimerID) override { fn_ = nullptr; } + void Fire() { + if (fn_) { + fn_(next_id_); + } + } + + private: + int32_t next_id_ = 0; + TimerCallback fn_ = nullptr; +}; + +} // namespace + +// Root cause: CFXGC_Platform overrides the no-arg GetForegroundTaskRunner() +// but NOT the priority-arg overload that the cppgc Sweeper actually calls +// (see v8/src/heap/cppgc/sweeper.cc, SweeperImpl::Start). So the Sweeper sees +// nullptr and can never post foreground finalization tasks. +TEST_F(HeapUnitTest, CFXGCPlatform_ForegroundRunnerOverrideMismatch) { + std::shared_ptr p = FXGC_CreatePlatformForTesting(); + ASSERT_TRUE(p); + // The no-arg overload is wired to the V8 platform's foreground runner. + EXPECT_NE(nullptr, p->GetForegroundTaskRunner()); + // The priority-arg overload falls through to the cppgc::Platform base + // implementation, which returns nullptr. This is the bug: any cppgc + // component that asks for a foreground runner *with* a priority gets + // nothing, so it silently degrades to "never schedule". + EXPECT_EQ(nullptr, + p->GetForegroundTaskRunner(cppgc::TaskPriority::kUserBlocking)) + << "BUG IS FIXED: priority overload now returns a runner"; + EXPECT_EQ(nullptr, + p->GetForegroundTaskRunner(cppgc::TaskPriority::kUserVisible)) + << "BUG IS FIXED: priority overload now returns a runner"; +} + +// Consequence: after a production-configured GC (kIncrementalAndConcurrent +// sweep, exactly what FXGC_CreateHeap configures), the destructors of dead +// finalizable objects never run because the Sweeper's +// ScheduleIncrementalSweeping() is a no-op without a foreground runner. +// This leaves CFX_Timer back-references in g_pwl_timer_map pointing into +// dead Oilpan memory across an unbounded window. +TEST_F(HeapUnitTest, CFXGCPlatform_SweeperNeverFinalizesDeadTimerOwners) { + CFX_Timer::InitializeGlobals(); + FakeTimerHandler handler; + GCedTimerOwner::s_live = 0; + GCedTimerOwner::s_fired = 0; + + // 1. Allocate the Oilpan object that registers a CFX_Timer back-ref. + cppgc::WeakPersistent weak = + cppgc::MakeGarbageCollected( + heap()->GetAllocationHandle(), &handler); + ASSERT_EQ(1, GCedTimerOwner::s_live); + + // 2. Drive a *production* GC: allocation pressure -> HeapGrowing -> + // GCInvoker posts a non-nestable task to the (no-arg) foreground runner + // -> Pump() runs it -> CollectGarbage with the heap's configured + // sweeping_support_ == kIncrementalAndConcurrent. Stop as soon as one + // GC has happened (weak handle cleared) so we don't trigger a *second* + // GC, which would FinishIfRunning() the sweeper synchronously. + for (int i = 0; i < 4096 && weak; ++i) { + cppgc::MakeGarbageCollected(heap()->GetAllocationHandle()); + Pump(); + } + ASSERT_FALSE(weak) << "allocation pressure never triggered a GC"; + + // 3. Give every posted task (including any that the Sweeper might have + // managed to post) a chance to run. + for (int i = 0; i < 64; ++i) { + Pump(); + } + + // 4. The destructor never ran: the Sweeper had nowhere to post the + // foreground SweepFinalizer task. The CFX_Timer is therefore still + // registered in g_pwl_timer_map with a callback_iface_ pointing into + // a dead, swept-but-unfinalized Oilpan slot. + EXPECT_EQ(1, GCedTimerOwner::s_live) + << "BUG IS FIXED: Sweeper finalized the dead object"; + + // 5. Prove the back-reference is live and reachable: firing the timer + // walks g_pwl_timer_map -> CFX_Timer -> callback_iface_ -> + // OnTimerFired() on the dead Oilpan object. + handler.Fire(); + EXPECT_EQ(1, GCedTimerOwner::s_fired); + + // Clean up so the fixture's atomic teardown GC doesn't trip on the + // dangling-at-destruction UnownedPtr check inside CFX_Timer. + FXGC_ForceGarbageCollection(heap()); + EXPECT_EQ(0, GCedTimerOwner::s_live); + CFX_Timer::DestroyGlobals(); +}