diff --git a/.agent/plans/resource_refactor_prerequisite_bugfixes.md b/.agent/plans/resource_refactor_prerequisite_bugfixes.md index d97ed9343..5c599e7b7 100644 --- a/.agent/plans/resource_refactor_prerequisite_bugfixes.md +++ b/.agent/plans/resource_refactor_prerequisite_bugfixes.md @@ -1,6 +1,6 @@ # Resource-refactor prerequisite bug fixes -Status: active defect track, 2026-07-12. +Status: active defect track, updated 2026-07-13. This document isolates correctness defects discovered during the shared-resource ownership audit. They should be fixed before the public resource API refactor wherever practical. Fixes in this track @@ -86,6 +86,10 @@ Regression coverage: - Repeat Engine creation/destruction in the same test process. - Assert no callback touches the destroyed scene/factory and no singleton is recreated. +Status: Engine now clears pool-owned HTTP clients before scene/Graphics teardown. The complete +producer barrier remains pending A2 and A4 because shared-executor operations and static UI +deliveries do not yet have complete close/reject semantics. + ### A4. UISceneNode static delivery queue lacks shutdown semantics Current behavior: @@ -122,21 +126,29 @@ Regression coverage: - Create/link programs, destroy Engine, and repeat under ASAN. +Status: fixed, 2026-07-13. ShaderProgramManager is destroyed before Renderer while the selected +window and context are still alive. + ### B2. TextLayout cache destroyed after FontManager Current behavior: Cached shaped glyphs retain raw FontTrueType pointers. The global TextLayout cache is currently -cleared after FontManager destruction. +cleared after FontManager destruction. `StaticLRU::clear()` also resets only its indexes, leaving +non-trivial cached values such as shared TextLayout pointers alive in its backing array. Fix: - Clear TextLayout and related shaped-font caches before FontManager. +- Release every active StaticLRU value when clearing the cache. Regression coverage: - Populate shaped-layout cache, destroy Engine, and verify repeated Engine lifecycle under ASAN. +Status: fixed, 2026-07-13. TextLayout is cleared before FontManager, and StaticLRU now resets its +active values. A weak cached layout expires during each tested Engine teardown. + ### B3. Scene/global resource manager order Current behavior: @@ -153,6 +165,10 @@ Regression coverage: - Destroy an Engine with live scene widgets using nine-patches and a non-empty batch. +Status: fixed, 2026-07-13. Scenes are destroyed first, BatchRenderer destruction explicitly drops +queued vertices and borrowed texture state without GL work, and global drawable/resource managers +remain alive until scene destruction completes. + ## 4. Priority C: loader and callback lifetime ### C0. MemoryManager first-use synchronization diff --git a/.agent/plans/resource_refactor_prerequisite_execution_plan.md b/.agent/plans/resource_refactor_prerequisite_execution_plan.md index 229da4abc..d8b2f5e73 100644 --- a/.agent/plans/resource_refactor_prerequisite_execution_plan.md +++ b/.agent/plans/resource_refactor_prerequisite_execution_plan.md @@ -1,6 +1,7 @@ # Resource-refactor prerequisite bug-fix execution plan -Status: active; work packages 1 and 2 completed, 2026-07-13. +Status: active; work packages 1 and 2 completed; Work Package 4 deterministic ordering implemented, +2026-07-13. This plan defines the bounded correctness work to complete before Stage 1 of the shared-resource ownership refactor. It turns the findings in `resource_refactor_prerequisite_bugfixes.md` into an @@ -143,6 +144,23 @@ Status: implemented. The focused TSAN suite initially reproduced the race in `MemoryManager::addPointer()`. After the fix, all six `ResourcePrerequisites` tests pass under TSAN without suppressions. The ASAN build and focused tests also pass. +### 3.6 StaticLRU clear retains non-trivial values + +Current defect found during Work Package 4 validation: + +`StaticLRU::clear()` resets its hash/list metadata and active count but leaves values in its backing +array. For `TextLayout::Cache`, this means `TextLayout::clearLayoutCache()` does not release cached +layouts or their raw font references. + +Fix: + +- Reset only the active value slots before clearing StaticLRU metadata. +- Keep the operation proportional to the number of live entries rather than total capacity. +- Verify cache release through a weak TextLayout handle during repeated Engine teardown. + +Status: implemented. The cached layout expires before FontManager destruction in both tested +Engine lifecycles. + Work-package exit criteria: - Each fix has an isolated regression test. @@ -295,6 +313,14 @@ Work-package exit criteria: - Every GPU-owning manager is destroyed while its required Renderer/context services remain valid. - Repeated Engine lifecycle tests pass. +Status: the deterministic dependency order is implemented. Engine clears pool-owned HTTP clients +first, makes the selected context current, destroys scenes, explicitly discards batch state, clears +TextLayout, releases high-level Graphics managers in dependency order, destroys shaders before +Renderer, and only then destroys windows/contexts. A focused test covers two Engine lifecycles with +a live UI scene, framebuffer, nine-patch, texture, font/layout cache, shaders, and pending batch. +The complete asynchronous-producer exit criterion remains pending Work Packages 3 and 5. +The complete ASAN unit suite passes: 746 tests passed and one opt-in visual test was skipped. + ## 7. Work package 5: UISceneNode async delivery lifecycle Current behavior: diff --git a/.agent/plans/resource_shared_ownership_stage0_inventory.md b/.agent/plans/resource_shared_ownership_stage0_inventory.md index ef3381492..150f2753e 100644 --- a/.agent/plans/resource_shared_ownership_stage0_inventory.md +++ b/.agent/plans/resource_shared_ownership_stage0_inventory.md @@ -342,6 +342,13 @@ TextLayout cache SystemFontResolver ``` +This list records the pre-prerequisite order found by the audit. On 2026-07-13 the deterministic +portion was corrected: pool-owned HTTP clients stop first; the selected context is made current; +scenes precede GlobalBatchRenderer and global resource managers; TextLayout precedes FontManager; +ShaderProgramManager precedes Renderer; and windows/contexts remain until all Graphics singleton +teardown is complete. Complete shared-executor and static UI-delivery barriers remain assigned to +their prerequisite work packages. + Concrete violations: | Current edge/order | Violation | diff --git a/include/eepp/core/lrucache.hpp b/include/eepp/core/lrucache.hpp index 2c1bc47bf..9a7277560 100644 --- a/include/eepp/core/lrucache.hpp +++ b/include/eepp/core/lrucache.hpp @@ -147,7 +147,9 @@ class StaticLRU { push_front( target_idx ); } - void clear() noexcept { + void clear() { + for ( std::size_t i = 0; i < used_; ++i ) + vals_[i] = ValueT{}; std::fill( table_.begin(), table_.end(), Entry{ NONE } ); std::fill( prev_.begin(), prev_.end(), NONE ); std::fill( next_.begin(), next_.end(), NONE ); diff --git a/include/eepp/graphics/batchrenderer.hpp b/include/eepp/graphics/batchrenderer.hpp index d14ec3bf1..b799f7b63 100644 --- a/include/eepp/graphics/batchrenderer.hpp +++ b/include/eepp/graphics/batchrenderer.hpp @@ -335,6 +335,9 @@ class EE_API BatchRenderer { void flush(); + /** Discard queued vertices and borrowed draw state without issuing graphics commands. */ + void discard(); + void init(); void addVertices( const unsigned int& num ); diff --git a/src/eepp/graphics/batchrenderer.cpp b/src/eepp/graphics/batchrenderer.cpp index 92c083aa9..781c98d96 100644 --- a/src/eepp/graphics/batchrenderer.cpp +++ b/src/eepp/graphics/batchrenderer.cpp @@ -25,6 +25,7 @@ BatchRenderer::BatchRenderer( const unsigned int& Prealloc ) { } BatchRenderer::~BatchRenderer() { + discard(); eeSAFE_DELETE_ARRAY( mVertex ); } @@ -48,6 +49,12 @@ void BatchRenderer::draw() { flush(); } +void BatchRenderer::discard() { + mNumVertex = 0; + mTVertex = nullptr; + mTexture = nullptr; +} + void BatchRenderer::setTexture( const Texture* texture, Texture::CoordinateType coordinateType ) { if ( mTexture != texture || mCoordinateType != coordinateType ) flush(); diff --git a/src/eepp/window/engine.cpp b/src/eepp/window/engine.cpp index 9e7ebe4db..57989545c 100644 --- a/src/eepp/window/engine.cpp +++ b/src/eepp/window/engine.cpp @@ -75,40 +75,52 @@ Engine::Engine() : Engine::~Engine() { mIsShuttingDown = true; - GlobalBatchRenderer::destroySingleton(); - - NinePatchManager::destroySingleton(); + // Stop and join pool-owned HTTP clients before any scene or graphics resource their callbacks + // can reach is destroyed. + Network::Http::Pool::getGlobal().clear(); + if ( mWindow ) + mWindow->setCurrent(); Scene::SceneManager::destroySingleton(); + // Scene destruction can leave borrowed textures in the batch. Destroying the batch explicitly + // discards those submissions while their resource managers are still alive. + GlobalBatchRenderer::destroySingleton(); + + // Cached layouts retain shaped runs that refer to managed fonts. + TextLayout::clearLayoutCache(); + CSS::StyleSheetSpecification::destroySingleton(); Doc::SyntaxDefinitionManager::destroySingleton(); + NinePatchManager::destroySingleton(); + FontManager::destroySingleton(); TextureAtlasManager::destroySingleton(); - TextureFactory::destroySingleton(); - - Graphics::Renderer::destroySingleton(); - - ShaderProgramManager::destroySingleton(); - - PackManager::destroySingleton(); - Graphics::Private::FrameBufferManager::destroySingleton(); Graphics::Private::VertexBufferManager::destroySingleton(); - VirtualFileSystem::destroySingleton(); + TextureFactory::destroySingleton(); + + // Shader and renderer destructors issue GL commands. Programs must go first while GLi and the + // current window context are still valid. + ShaderProgramManager::destroySingleton(); + + Graphics::Renderer::destroySingleton(); + + PackManager::destroySingleton(); #ifdef EE_SSL_SUPPORT Network::SSL::SSLSocket::end(); #endif - Network::Http::Pool::getGlobal().clear(); + VirtualFileSystem::destroySingleton(); + // Windows own the GL contexts and must outlive every GPU resource and graphics manager above. destroy(); #if EE_PLATFORM == EE_PLATFORM_ANDROID @@ -121,15 +133,13 @@ Engine::~Engine() { eeSAFE_DELETE( mBackend ); + SystemFontResolver::destroySingleton(); + RegExCache::destroySingleton(); ParserMatcherManager::destroySingleton(); Log::destroySingleton(); - - TextLayout::clearLayoutCache(); - - SystemFontResolver::destroySingleton(); } void Engine::destroy() { diff --git a/src/tests/unit_tests/resource_prerequisite_tests.cpp b/src/tests/unit_tests/resource_prerequisite_tests.cpp index 06ef5240d..5a9de5fc8 100644 --- a/src/tests/unit_tests/resource_prerequisite_tests.cpp +++ b/src/tests/unit_tests/resource_prerequisite_tests.cpp @@ -2,11 +2,25 @@ #include #include +#include +#include +#include +#include +#include +#include +#include +#include #include #include +#include #include +#include +#include #include +#include #include +#include +#include #include #include #include @@ -17,6 +31,8 @@ using namespace EE; using namespace EE::Graphics; +using namespace EE::Scene; +using namespace EE::UI; using namespace EE::UI::Models; using namespace EE::Window; @@ -173,3 +189,50 @@ UTEST( ResourcePrerequisites, textureAtlasLoaderWaitsBeforeDestroyingCallbackSta EXPECT_TRUE( *callbackCompleted ); } + +UTEST( ResourcePrerequisites, engineTeardownReleasesGraphicsBeforeContextsAcrossRestarts ) { + for ( int cycle = 0; cycle < 2; ++cycle ) { + Engine::instance()->createWindow( + WindowSettings( 64, 64, "Engine teardown test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, true ), + ContextSettings( false, 0, 0, GLv_default, true, false ) ); + + Texture* texture = TextureFactory::instance()->createEmptyTexture( 4, 4 ); + ASSERT_TRUE( texture != nullptr ); + + NinePatch* ninePatch = NinePatchManager::instance()->add( + NinePatch::New( texture, 1, 1, 1, 1, 1, "engine-teardown-nine-patch" ) ); + ASSERT_TRUE( ninePatch != nullptr ); + + auto* scene = UISceneNode::New(); + SceneManager::instance()->add( scene ); + scene->enableFrameBuffer(); + UIImage::New()->setDrawable( ninePatch )->setParent( scene->getRoot() ); + + auto* font = FontTrueType::New( "engine-teardown-font" ); + ASSERT_TRUE( + font->loadFromFile( Sys::getProcessPath() + "../assets/fonts/NotoSans-Regular.ttf" ) ); + auto layout = TextLayout::layout( String( "cached before Engine teardown" ), font, 14, 0 ); + std::weak_ptr layoutWeak = layout; + layout.reset(); + + auto* batch = GlobalBatchRenderer::instance(); + batch->setTexture( texture ); + batch->batchQuad( 0, 0, 4, 4 ); + + Engine::destroySingleton(); + + EXPECT_TRUE( layoutWeak.expired() ); + EXPECT_TRUE( Engine::existsSingleton() == nullptr ); + EXPECT_TRUE( SceneManager::existsSingleton() == nullptr ); + EXPECT_TRUE( GlobalBatchRenderer::existsSingleton() == nullptr ); + EXPECT_TRUE( NinePatchManager::existsSingleton() == nullptr ); + EXPECT_TRUE( FontManager::existsSingleton() == nullptr ); + EXPECT_TRUE( TextureAtlasManager::existsSingleton() == nullptr ); + EXPECT_TRUE( TextureFactory::existsSingleton() == nullptr ); + EXPECT_TRUE( ShaderProgramManager::existsSingleton() == nullptr ); + EXPECT_TRUE( Graphics::Private::FrameBufferManager::existsSingleton() == nullptr ); + EXPECT_TRUE( Graphics::Private::VertexBufferManager::existsSingleton() == nullptr ); + EXPECT_TRUE( Renderer::existsSingleton() == nullptr ); + } +}