Implemented the deterministic Engine teardown ordering in Engine destructor:

- Pool-owned HTTP clients stop first.
  - The active GL context is made current.
  - Scenes are destroyed before batches and resource managers.
  - Pending batches explicitly discard vertices and borrowed textures.
  - TextLayout cache clears before fonts.
  - Framebuffer/vertex managers and textures are released while Renderer/context remain alive.
  - ShaderProgramManager is destroyed before Renderer.
  - Windows and contexts are destroyed last.

The lifecycle test uncovered and fixed another bug: StaticLRU::clear() retained non-trivial values such as cached shared_ptr<TextLayout> objects. It now releases active values in LRUCache.
This commit is contained in:
Martín Lucas Golini
2026-07-14 00:03:23 -03:00
parent 513ef0f12b
commit 8aa660104f
8 changed files with 155 additions and 21 deletions
@@ -1,6 +1,6 @@
# Resource-refactor prerequisite bug fixes # 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. 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 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. - Repeat Engine creation/destruction in the same test process.
- Assert no callback touches the destroyed scene/factory and no singleton is recreated. - 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 ### A4. UISceneNode static delivery queue lacks shutdown semantics
Current behavior: Current behavior:
@@ -122,21 +126,29 @@ Regression coverage:
- Create/link programs, destroy Engine, and repeat under ASAN. - 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 ### B2. TextLayout cache destroyed after FontManager
Current behavior: Current behavior:
Cached shaped glyphs retain raw FontTrueType pointers. The global TextLayout cache is currently 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: Fix:
- Clear TextLayout and related shaped-font caches before FontManager. - Clear TextLayout and related shaped-font caches before FontManager.
- Release every active StaticLRU value when clearing the cache.
Regression coverage: Regression coverage:
- Populate shaped-layout cache, destroy Engine, and verify repeated Engine lifecycle under ASAN. - 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 ### B3. Scene/global resource manager order
Current behavior: Current behavior:
@@ -153,6 +165,10 @@ Regression coverage:
- Destroy an Engine with live scene widgets using nine-patches and a non-empty batch. - 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 ## 4. Priority C: loader and callback lifetime
### C0. MemoryManager first-use synchronization ### C0. MemoryManager first-use synchronization
@@ -1,6 +1,7 @@
# Resource-refactor prerequisite bug-fix execution plan # 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 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 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 `MemoryManager::addPointer()`. After the fix, all six `ResourcePrerequisites` tests pass under
TSAN without suppressions. The ASAN build and focused tests also pass. 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: Work-package exit criteria:
- Each fix has an isolated regression test. - 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. - Every GPU-owning manager is destroyed while its required Renderer/context services remain valid.
- Repeated Engine lifecycle tests pass. - 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 ## 7. Work package 5: UISceneNode async delivery lifecycle
Current behavior: Current behavior:
@@ -342,6 +342,13 @@ TextLayout cache
SystemFontResolver 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: Concrete violations:
| Current edge/order | Violation | | Current edge/order | Violation |
+3 -1
View File
@@ -147,7 +147,9 @@ class StaticLRU {
push_front( target_idx ); 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( table_.begin(), table_.end(), Entry{ NONE } );
std::fill( prev_.begin(), prev_.end(), NONE ); std::fill( prev_.begin(), prev_.end(), NONE );
std::fill( next_.begin(), next_.end(), NONE ); std::fill( next_.begin(), next_.end(), NONE );
+3
View File
@@ -335,6 +335,9 @@ class EE_API BatchRenderer {
void flush(); void flush();
/** Discard queued vertices and borrowed draw state without issuing graphics commands. */
void discard();
void init(); void init();
void addVertices( const unsigned int& num ); void addVertices( const unsigned int& num );
+7
View File
@@ -25,6 +25,7 @@ BatchRenderer::BatchRenderer( const unsigned int& Prealloc ) {
} }
BatchRenderer::~BatchRenderer() { BatchRenderer::~BatchRenderer() {
discard();
eeSAFE_DELETE_ARRAY( mVertex ); eeSAFE_DELETE_ARRAY( mVertex );
} }
@@ -48,6 +49,12 @@ void BatchRenderer::draw() {
flush(); flush();
} }
void BatchRenderer::discard() {
mNumVertex = 0;
mTVertex = nullptr;
mTexture = nullptr;
}
void BatchRenderer::setTexture( const Texture* texture, Texture::CoordinateType coordinateType ) { void BatchRenderer::setTexture( const Texture* texture, Texture::CoordinateType coordinateType ) {
if ( mTexture != texture || mCoordinateType != coordinateType ) if ( mTexture != texture || mCoordinateType != coordinateType )
flush(); flush();
+27 -17
View File
@@ -75,40 +75,52 @@ Engine::Engine() :
Engine::~Engine() { Engine::~Engine() {
mIsShuttingDown = true; mIsShuttingDown = true;
GlobalBatchRenderer::destroySingleton(); // Stop and join pool-owned HTTP clients before any scene or graphics resource their callbacks
// can reach is destroyed.
NinePatchManager::destroySingleton(); Network::Http::Pool::getGlobal().clear();
if ( mWindow )
mWindow->setCurrent();
Scene::SceneManager::destroySingleton(); 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(); CSS::StyleSheetSpecification::destroySingleton();
Doc::SyntaxDefinitionManager::destroySingleton(); Doc::SyntaxDefinitionManager::destroySingleton();
NinePatchManager::destroySingleton();
FontManager::destroySingleton(); FontManager::destroySingleton();
TextureAtlasManager::destroySingleton(); TextureAtlasManager::destroySingleton();
TextureFactory::destroySingleton();
Graphics::Renderer::destroySingleton();
ShaderProgramManager::destroySingleton();
PackManager::destroySingleton();
Graphics::Private::FrameBufferManager::destroySingleton(); Graphics::Private::FrameBufferManager::destroySingleton();
Graphics::Private::VertexBufferManager::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 #ifdef EE_SSL_SUPPORT
Network::SSL::SSLSocket::end(); Network::SSL::SSLSocket::end();
#endif #endif
Network::Http::Pool::getGlobal().clear(); VirtualFileSystem::destroySingleton();
// Windows own the GL contexts and must outlive every GPU resource and graphics manager above.
destroy(); destroy();
#if EE_PLATFORM == EE_PLATFORM_ANDROID #if EE_PLATFORM == EE_PLATFORM_ANDROID
@@ -121,15 +133,13 @@ Engine::~Engine() {
eeSAFE_DELETE( mBackend ); eeSAFE_DELETE( mBackend );
SystemFontResolver::destroySingleton();
RegExCache::destroySingleton(); RegExCache::destroySingleton();
ParserMatcherManager::destroySingleton(); ParserMatcherManager::destroySingleton();
Log::destroySingleton(); Log::destroySingleton();
TextLayout::clearLayoutCache();
SystemFontResolver::destroySingleton();
} }
void Engine::destroy() { void Engine::destroy() {
@@ -2,11 +2,25 @@
#include <atomic> #include <atomic>
#include <condition_variable> #include <condition_variable>
#include <eepp/graphics/fontmanager.hpp>
#include <eepp/graphics/fonttruetype.hpp>
#include <eepp/graphics/framebuffermanager.hpp>
#include <eepp/graphics/globalbatchrenderer.hpp>
#include <eepp/graphics/ninepatchmanager.hpp>
#include <eepp/graphics/renderer/renderer.hpp>
#include <eepp/graphics/shaderprogrammanager.hpp>
#include <eepp/graphics/textlayout.hpp>
#include <eepp/graphics/textureatlas.hpp> #include <eepp/graphics/textureatlas.hpp>
#include <eepp/graphics/textureatlasloader.hpp> #include <eepp/graphics/textureatlasloader.hpp>
#include <eepp/graphics/textureatlasmanager.hpp>
#include <eepp/graphics/texturefactory.hpp> #include <eepp/graphics/texturefactory.hpp>
#include <eepp/graphics/vertexbuffermanager.hpp>
#include <eepp/scene/scenemanager.hpp>
#include <eepp/system/resourceloader.hpp> #include <eepp/system/resourceloader.hpp>
#include <eepp/system/sys.hpp>
#include <eepp/ui/models/variant.hpp> #include <eepp/ui/models/variant.hpp>
#include <eepp/ui/uiimage.hpp>
#include <eepp/ui/uiscenenode.hpp>
#include <eepp/window/engine.hpp> #include <eepp/window/engine.hpp>
#include <eepp/window/window.hpp> #include <eepp/window/window.hpp>
#include <limits> #include <limits>
@@ -17,6 +31,8 @@
using namespace EE; using namespace EE;
using namespace EE::Graphics; using namespace EE::Graphics;
using namespace EE::Scene;
using namespace EE::UI;
using namespace EE::UI::Models; using namespace EE::UI::Models;
using namespace EE::Window; using namespace EE::Window;
@@ -173,3 +189,50 @@ UTEST( ResourcePrerequisites, textureAtlasLoaderWaitsBeforeDestroyingCallbackSta
EXPECT_TRUE( *callbackCompleted ); 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<const TextLayout> 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 );
}
}