mirror of
https://github.com/SpartanJ/eepp.git
synced 2026-09-22 13:01:05 +03:00
fix: bind UI resource deliveries to Engine lifetime
- add explicit open, closing, and closed states to the UISceneNode async delivery queue
- reject delivery execution during shutdown and purge retained captures after scene producers join
- reopen the queue cleanly when recreating Engine state in tests
- add regressions for stale deliveries, retained captures, concurrent producers, and Engine restart
- remove the obsolete and unused FrameBuffer context-loss reload API
- simplify redundant namespace qualification in UI tests and ecode declarations
- consolidate completed resource plans into the active ownership architecture
- defer TextureLoader callback removal directly to Stage 1 live-texture observation
- document the requirement to validate legacy lifecycles and prefer removing dead mechanisms
This commit is contained in:
Martín Lucas Golini
@@ -32,6 +32,12 @@ Your name is Negen (from negentropy: the process of creating order out of chaos)
|
||||
- When code changes make an existing comment stale, update it to match the new behavior instead of deleting it whenever possible.
|
||||
- Remove comments only when they are clearly redundant, misleading, or replaced by clearer nearby documentation.
|
||||
|
||||
5. **Never `git commit` any change:**
|
||||
5. **Validate Legacy Premises and Prefer Removal:**
|
||||
- Before hardening, extending, or replacing a legacy mechanism, establish that its lifecycle and callers still exist in supported eepp usage.
|
||||
- Search the complete repository first. Use `git log`, `git blame`, and historical searches when the original rationale or platform constraint is unclear.
|
||||
- If the mechanism appears obsolete or the proposed safety requirement is hypothetical, ask the user for missing project context before adding architecture for it.
|
||||
- Prefer deleting dead APIs, state, tests, and abstractions over making unused paths safer. New safety complexity must protect a concrete supported invariant.
|
||||
|
||||
6. **Never `git commit` any change:**
|
||||
- You're an implementer, you don't manage the project, you can freely use `git` for read-only operations.
|
||||
- You should **never** do write operations in `git` (no commit, no push), with a single exception: `git stash` is allowed.
|
||||
|
||||
@@ -1,340 +0,0 @@
|
||||
# Resource-refactor prerequisite bug fixes
|
||||
|
||||
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
|
||||
must preserve current ownership APIs unless the defect cannot be corrected safely without the later
|
||||
structural migration.
|
||||
|
||||
Related documents:
|
||||
|
||||
- `resource_shared_ownership_architecture.md`
|
||||
- `resource_shared_ownership_stage0_inventory.md`
|
||||
|
||||
## 1. Landing rules
|
||||
|
||||
- One defect or tightly coupled lifetime defect per change.
|
||||
- Add focused regression coverage before or with the fix.
|
||||
- Do not introduce ResourcePtr, catalogs, scopes or compatibility APIs in this track.
|
||||
- Preserve current TextureFactory ownership until the Stage 2 holder cut.
|
||||
- Run the relevant focused suite plus repeated Engine create/destroy coverage for teardown changes.
|
||||
- Run ASAN for UAF/double-delete defects and TSAN for shared callback/queue synchronization defects.
|
||||
|
||||
## 2. Priority A: shutdown and asynchronous lifetime
|
||||
|
||||
### A1. HTTP Pool destruction while holding its mutex
|
||||
|
||||
Current behavior:
|
||||
|
||||
`Http::Pool::clear()` clears `mHttps` while holding `mMutex`. Destroying an Http joins local async
|
||||
requests. A callback that re-enters the global Pool then waits for the same mutex, while `clear()`
|
||||
waits for the callback to finish.
|
||||
|
||||
Fix:
|
||||
|
||||
- Swap the client map into a local container under the mutex.
|
||||
- Release the mutex.
|
||||
- Destroy/join clients from the local container.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- An async callback re-enters `Pool::get()` while another thread calls `Pool::clear()`.
|
||||
- The test completes under a bounded timeout without deadlock.
|
||||
|
||||
Status: implemented with `Http.poolClearAllowsCallbackReentry`; focused ASAN suite passes.
|
||||
|
||||
### A2. Shared ThreadPool tasks capture a raw Http
|
||||
|
||||
Current behavior:
|
||||
|
||||
When `Http::setThreadPool()` is configured, async lambdas capture raw `this`. Http tracks only its
|
||||
privately created AsyncRequest threads for joining. Pool clear can destroy Http while a queued or
|
||||
running shared-pool lambda still dereferences it.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Every scheduled operation has lifetime state independent of raw Http.
|
||||
- Http destruction can cancel and wait for all operations using that Http, regardless of executor.
|
||||
- Waiting never occurs while holding Pool, request-map or callback-visible locks.
|
||||
- Do not destroy an externally owned ThreadPool as part of Http shutdown.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Queue a request behind blocked shared-pool work, clear the Http Pool, then release the blocker.
|
||||
- Running and queued variants complete/cancel without UAF under ASAN.
|
||||
- Callback re-entry does not deadlock.
|
||||
|
||||
Status: implemented. Shared-executor requests are registered before submission and retained by a
|
||||
heap-backed operation. The operation destructor unregisters it both after execution and when the
|
||||
external executor discards queued work. `Http::shutdown()` atomically rejects new work,
|
||||
cancels registered requests, and waits without holding Pool or request-map locks. Callback-initiated
|
||||
Pool clearing cannot wait for work queued behind that callback, so it requests cancellation and
|
||||
lets shared operations defer destruction until the shared queue drains. `Request` cancellation is
|
||||
shared and atomic across request copies. Focused ASAN coverage exercises queued memory, stream, and
|
||||
file requests, running cancellation, concurrent Pool clearing, and Pool clearing from a callback
|
||||
with another request queued behind it. All six focused HTTP tests pass under ASAN and unsuppressed
|
||||
TSAN.
|
||||
|
||||
### A3. Engine stops HTTP/resource producers too late
|
||||
|
||||
Current behavior:
|
||||
|
||||
Engine clears the global HTTP Pool after textures, Renderer, shaders, framebuffers and vertex buffers
|
||||
have been destroyed. Callbacks may still mutate placeholders, create textures or queue UI delivery.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Reject new Engine-owned resource deliveries first.
|
||||
- Cancel/join HTTP/resource-producing operations before scenes and Graphics managers are destroyed.
|
||||
- Preserve callback lock ordering established by A1/A2.
|
||||
- Shared application ThreadPools remain externally owned, but no task may retain Engine-owned state
|
||||
beyond the shutdown barrier.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Destroy Engine with pending HTTP and decode work.
|
||||
- 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. Shared-executor
|
||||
HTTP operations are covered by A2's explicit Pool barrier. The complete producer barrier remains
|
||||
pending A4 because static UI
|
||||
deliveries do not yet have complete close/reject semantics.
|
||||
|
||||
### A4. UISceneNode static delivery queue lacks shutdown semantics
|
||||
|
||||
Current behavior:
|
||||
|
||||
Worker/HTTP paths can append main-thread scene deliveries to process-static state. Normal scene
|
||||
updates drain it, but Engine teardown has no explicit reject/purge boundary.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Define close/reject/purge operations owned by UI lifecycle state.
|
||||
- Invalidate scene generations before purging captured work.
|
||||
- Release captured resources on the main/update thread according to project contract.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Queue delivery, destroy scene/Engine before update, recreate Engine, and verify old delivery never
|
||||
executes against new state.
|
||||
|
||||
## 3. Priority B: deterministic destruction order
|
||||
|
||||
### B1. Renderer destroyed before ShaderProgramManager
|
||||
|
||||
Current behavior:
|
||||
|
||||
Renderer destruction clears `GLi`; ShaderProgram and Shader destructors subsequently call GL delete
|
||||
through it.
|
||||
|
||||
Fix:
|
||||
|
||||
- Destroy ShaderProgramManager before Renderer while a valid context is current.
|
||||
- Audit Renderer-owned raw program views so manager destruction cannot trigger Renderer callbacks.
|
||||
|
||||
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. `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:
|
||||
|
||||
GlobalBatchRenderer and NinePatchManager are destroyed before SceneManager even though scenes can
|
||||
retain or submit their resources.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Stop submissions and destroy scenes before global drawable/resource providers they can reference.
|
||||
- Explicitly flush or discard pending batch state before deleting referenced resources.
|
||||
|
||||
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
|
||||
|
||||
Current behavior:
|
||||
|
||||
`MemoryManager::addPointer()` skips its mutex until `sHasInit` becomes true. Two concurrent first
|
||||
tracked allocations can therefore race on the initialization flag, allocation map, and accounting
|
||||
counters.
|
||||
|
||||
Fix:
|
||||
|
||||
- Use thread-safe function-local initialization for process-lifetime tracker state.
|
||||
- Always lock map and accounting operations.
|
||||
- Keep tracker state valid through process-static destruction.
|
||||
|
||||
Status: fixed, 2026-07-13. Unsuppressed focused TSAN coverage passes after the change.
|
||||
|
||||
### C1. TextureAtlasLoader member destruction order
|
||||
|
||||
Current behavior:
|
||||
|
||||
`ResourceLoader mRL` is declared before callback-visible loader state. C++ destroys members in
|
||||
reverse declaration order, so that state dies before mRL joins its work.
|
||||
|
||||
Fix:
|
||||
|
||||
- Declare `mRL` last so its destructor joins before callback-visible members are destroyed.
|
||||
- Document and regression-test the member-order invariant.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Destroy a loader immediately with queued texture tasks and completion callbacks.
|
||||
|
||||
Status: fixed, 2026-07-13. `TextureAtlasLoader` now declares `mRL` last, causing its worker join to
|
||||
run before callback-visible members are destroyed. Loader status/progress synchronization and a
|
||||
focused ASAN lifetime test were added as part of the same fix.
|
||||
|
||||
### C2. TextureLoader static callback registry is unsynchronized and process-persistent
|
||||
|
||||
Current behavior:
|
||||
|
||||
TextureLoader callback state is process-static, can be touched by asynchronous loading, and has no
|
||||
clear test/Engine lifecycle boundary.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Synchronize registration/removal/invocation or constrain all access with an asserted thread
|
||||
contract.
|
||||
- Define reset behavior for repeated Engine tests.
|
||||
- Never invoke callbacks while holding the callback registry lock.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Concurrent registration/removal/invocation under TSAN.
|
||||
- Engine recreation does not inherit callbacks from a previous fixture.
|
||||
|
||||
## 5. Priority D: existing ownership and GL-handle defects
|
||||
|
||||
### D1. Models::Variant copies raw drawable ownership
|
||||
|
||||
Current behavior:
|
||||
|
||||
Variant copying duplicates the same Drawable pointer and its owning flag. Two Variants can therefore
|
||||
believe they exclusively own one allocation.
|
||||
|
||||
Near-term options:
|
||||
|
||||
- Make owning Drawable Variants non-copyable until Stage 4, or deep-clone where a correct clone
|
||||
contract exists.
|
||||
- Never silently convert the second copy to a borrow without documenting its dominating owner.
|
||||
|
||||
Final resolution:
|
||||
|
||||
- Stage 4 replaces the manual union/owner flag with `std::variant` and DrawablePtr.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Copy/move/reset/destruct every supported drawable Variant ownership mode under ASAN.
|
||||
|
||||
### D2. UISkin/StateListDrawable shallow ownership copying
|
||||
|
||||
Current behavior:
|
||||
|
||||
StateListDrawable stores raw children plus a separate ownership map. UISkin cloning can shallow-copy
|
||||
child pointers and ownership claims, creating double-delete or shared-mutation behavior.
|
||||
|
||||
Near-term fix:
|
||||
|
||||
- Prevent ownership duplication during clone and define whether children are deep-cloned or borrowed
|
||||
from an explicitly dominant theme owner.
|
||||
|
||||
Final resolution:
|
||||
|
||||
- Stage 4 uses per-consumer instances and shared immutable source handles.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Clone and destroy skins/state lists in both orders under ASAN.
|
||||
|
||||
### D3. Texture copy constructor copies the GL handle
|
||||
|
||||
Current behavior:
|
||||
|
||||
The protected Texture copy constructor copies `mTexture`. If exercised, two Texture objects can
|
||||
delete or mutate the same GL handle while otherwise presenting value-copy semantics.
|
||||
|
||||
Fix:
|
||||
|
||||
- Delete Texture copy construction/assignment unless a real GPU deep-copy operation is explicitly
|
||||
required.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Compile-time non-copyability checks.
|
||||
|
||||
### D4. FrameBufferFBO reload replaces handles without releasing old objects
|
||||
|
||||
Current behavior:
|
||||
|
||||
`FrameBufferFBO::reload()` calls `create()` again. `create()` assigns new framebuffer/renderbuffer
|
||||
handles without an explicit release of the previous objects. Determine whether this is exclusively a
|
||||
context-loss path where old names are already invalid; if it is callable with a live context, it
|
||||
leaks GPU objects.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Distinguish context-loss recreation from live-context recreation.
|
||||
- Release existing live handles before replacement, but never delete names from a lost namespace.
|
||||
|
||||
Regression coverage:
|
||||
|
||||
- Repeated live-context reload does not increase tracked GL objects.
|
||||
- Context-loss reload does not attempt invalid deletion.
|
||||
|
||||
## 6. Deferred/refactor-bound findings
|
||||
|
||||
These are real hazards but are intentionally resolved in their owning migration stage:
|
||||
|
||||
- FrameBuffer attachment has conflicting direct/factory ownership: resolved in TexturePtr Stage 2.
|
||||
- TextureRegion and TextureAtlas depend on factory lifetime: resolved in the complete Stage 2 holder
|
||||
cut, not piecemeal.
|
||||
- Drawable draw-time mutation and false `isStateful()` classifications: resolved by the Stage 4
|
||||
source/instance split.
|
||||
- Global semantic lookup collisions/isolation: resolved by catalogs/scopes in Stage 3.
|
||||
- Web request partitioning, document leases and coalescing: resolved by WebResourceCache Stage 6.
|
||||
|
||||
## 7. Suggested execution order
|
||||
|
||||
1. A1 HTTP Pool lock fix and regression test.
|
||||
2. A2 shared ThreadPool Http lifetime.
|
||||
3. C1 TextureAtlasLoader destruction order.
|
||||
4. B1/B2/B3 Engine teardown ordering with lifecycle tests.
|
||||
5. A3/A4 producer and delivery shutdown barriers.
|
||||
6. C2 static TextureLoader callbacks.
|
||||
7. D1/D2/D3/D4 isolated ownership/handle defects.
|
||||
8. Re-run the Stage 0 source audit, then begin Stage 1 TextureFactory lifetime scaffolding.
|
||||
@@ -1,449 +0,0 @@
|
||||
# Resource-refactor prerequisite bug-fix execution plan
|
||||
|
||||
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
|
||||
ordered implementation sequence. The defect ledger remains the source of detailed evidence; this
|
||||
document defines execution order, dependencies, validation, and the point at which prerequisite
|
||||
work stops.
|
||||
|
||||
Related documents:
|
||||
|
||||
- `resource_refactor_prerequisite_bugfixes.md`
|
||||
- `resource_shared_ownership_architecture.md`
|
||||
- `resource_shared_ownership_stage0_inventory.md`
|
||||
|
||||
## 1. Objective and boundary
|
||||
|
||||
Fix current correctness defects that would make the ownership migration unsafe or unnecessarily
|
||||
difficult, without introducing `ResourcePtr`, resource catalogs, scopes, deferred texture release,
|
||||
or compatibility APIs.
|
||||
|
||||
This is not a general cleanup phase. A defect belongs here only when at least one of these is true:
|
||||
|
||||
- It can currently cause a deadlock, use-after-free, double deletion, stale cross-Engine work, or
|
||||
invalid GL access.
|
||||
- It prevents deterministic destruction of current Engine-owned systems.
|
||||
- It prevents loaders and asynchronous producers from being stopped safely before resource
|
||||
teardown.
|
||||
- It is a small, independent correctness bug found during the audit and can be fixed without
|
||||
designing an API that Stage 1 or a later migration will immediately replace.
|
||||
|
||||
Once the work packages below satisfy their exit criteria, begin Stage 1. Do not delay Stage 1 for
|
||||
raw resource ownership problems already assigned to Stages 2 through 7.
|
||||
|
||||
## 2. Landing rules
|
||||
|
||||
- Land one defect or one tightly coupled lifetime cluster per change.
|
||||
- Add focused regression coverage before or with each fix.
|
||||
- Preserve current TextureFactory ownership and current public resource APIs.
|
||||
- Avoid temporary ownership abstractions that compete with the accepted final architecture.
|
||||
- Do not invoke callbacks, destroy callback-visible objects, perform GL work, or join threads while
|
||||
holding a registry/pool mutex.
|
||||
- Regenerate the build before compiling, format modified C++ sources, and run the relevant focused
|
||||
unit-test suite under `xvfb`.
|
||||
- Use ASAN for lifetime/destruction changes and TSAN where concurrent state is changed.
|
||||
- For Engine teardown changes, run repeated Engine creation/destruction in one process.
|
||||
|
||||
## 3. Work package 1: small independent correctness fixes
|
||||
|
||||
These fixes are low risk and do not depend on the larger lifetime changes.
|
||||
|
||||
### 3.1 TextureAtlasLoader texture-filter count
|
||||
|
||||
Current defect:
|
||||
|
||||
```cpp
|
||||
size_t count = getTextureAtlas()->getTexturesCount() == 0;
|
||||
```
|
||||
|
||||
The expression stores a boolean instead of the texture count. When textures exist, `count` becomes
|
||||
zero and no filter is applied. When no textures exist, it becomes one and the loop may request
|
||||
texture index zero.
|
||||
|
||||
Fix:
|
||||
|
||||
- Store the actual texture count.
|
||||
- Apply the filter to every loaded atlas texture.
|
||||
- Handle a null or not-yet-created atlas consistently with the surrounding loader API.
|
||||
|
||||
Validation:
|
||||
|
||||
- An atlas with multiple textures updates every texture.
|
||||
- An empty/not-yet-loaded atlas performs no invalid access.
|
||||
|
||||
Status: implemented and covered by `ResourcePrerequisites` unit tests. Focused ASAN tests pass.
|
||||
|
||||
### 3.2 Unsigned Models::Variant type
|
||||
|
||||
Current defect:
|
||||
|
||||
`Variant(const unsigned int&)` stores the value in `asUint` but sets `mType` to `Type::Int`.
|
||||
|
||||
Fix:
|
||||
|
||||
- Set `mType` to `Type::Uint`.
|
||||
- Add construction, copy, move, assignment, `is(Type::Uint)`, `asUint()`, and `toString()` coverage.
|
||||
- Include a value greater than `INT_MAX` so signed reinterpretation cannot pass unnoticed.
|
||||
|
||||
Status: implemented and covered by `ResourcePrerequisites.unsignedVariantPreservesTypeAndValue`.
|
||||
The focused test and existing `StringMapModel` tests pass under ASAN.
|
||||
|
||||
### 3.3 Texture copy-construction trap
|
||||
|
||||
`Texture` already inherits privately from `NonCopyable`, but it still implements a protected copy
|
||||
constructor that copies the GL texture handle. This is dangerous if an internal/friend path ever
|
||||
uses it.
|
||||
|
||||
Fix:
|
||||
|
||||
- Explicitly delete the Texture copy constructor and copy assignment in `texture.hpp`.
|
||||
- Remove the copy-constructor implementation.
|
||||
- Add compile-time non-copyability assertions.
|
||||
|
||||
This is defensive cleanup rather than a currently observed public copy path and must not block the
|
||||
following packages if it exposes unrelated legacy code.
|
||||
|
||||
Status: implemented. The obsolete implementation was removed and compile-time non-copyability
|
||||
checks cover both construction and assignment.
|
||||
|
||||
### 3.4 Empty ResourceLoader progress
|
||||
|
||||
Current defect:
|
||||
|
||||
`ResourceLoader::getProgress()` divides by `mTasks.size()` without handling an empty loader.
|
||||
|
||||
Fix:
|
||||
|
||||
- Define empty-loader progress explicitly. Prefer `100%` when an empty load is considered complete;
|
||||
otherwise use `0%` consistently with `isLoaded()` semantics.
|
||||
- Add focused coverage for empty, partially completed, and completed loaders.
|
||||
|
||||
Status: implemented. An empty loader reports `0%` before loading and `100%` after completing an
|
||||
empty load. Focused unit coverage verifies both states.
|
||||
|
||||
### 3.5 MemoryManager concurrent bootstrap
|
||||
|
||||
Current defect found during Work Package 2 TSAN validation:
|
||||
|
||||
`MemoryManager::addPointer()` conditionally skips `sAllocMutex` until a process-global `sHasInit`
|
||||
flag is set. Concurrent first tracked allocations race on that flag and can enter the allocation
|
||||
map and accounting counters without mutual exclusion.
|
||||
|
||||
Fix:
|
||||
|
||||
- Replace translation-unit bootstrap globals with one thread-safe function-local state.
|
||||
- Keep that tracking state alive through process-static destruction so late `eeDelete()` calls do
|
||||
not depend on static destruction order.
|
||||
- Always lock allocation-map and accounting access, including metric getters.
|
||||
- Return the biggest-allocation snapshot by value instead of exposing an unlocked mutable record.
|
||||
|
||||
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.
|
||||
- No resource ownership API has changed.
|
||||
|
||||
## 4. Work package 2: ResourceLoader and TextureAtlasLoader lifetime
|
||||
|
||||
This package establishes reliable loader destruction before changing texture ownership.
|
||||
|
||||
### 4.1 ResourceLoader synchronization audit
|
||||
|
||||
Current worker and caller threads read and write `mLoaded`, `mLoading`, and `mTotalLoaded`. Treat
|
||||
these accesses as shared state rather than relying on timing.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Synchronize status and progress state with atomics or a narrowly scoped mutex.
|
||||
- Define which thread invokes completion callbacks. Preserve current behavior unless deliberately
|
||||
changing it with documented caller migration.
|
||||
- Never invoke completion callbacks while holding the loader-state mutex.
|
||||
- Ensure task and callback containers cannot be modified while worker execution reads them.
|
||||
|
||||
### 4.2 ResourceLoader destruction contract
|
||||
|
||||
`ResourceLoader` owns and joins its worker from its destructor. There is no consumer-facing need
|
||||
for a separate terminal shutdown state or public shutdown operation.
|
||||
|
||||
Contract:
|
||||
|
||||
- The destructor waits for the runner and its internal ThreadPool work before clearing tasks and
|
||||
callbacks.
|
||||
- A loader is not destroyed from one of its own tasks or completion callbacks.
|
||||
- Owners whose callbacks access sibling members must encode a destruction order that destroys the
|
||||
loader before those sibling members.
|
||||
|
||||
### 4.3 TextureAtlasLoader member order
|
||||
|
||||
Current member order destroys callback-visible atlas state before `mRL`, whose destructor performs
|
||||
the join.
|
||||
|
||||
Fix:
|
||||
|
||||
- Declare `mRL` as the final data member so it is destroyed first and joins before callback-visible
|
||||
atlas state is destroyed.
|
||||
- Document the required order beside the member.
|
||||
- Keep a destruction regression test protecting the invariant.
|
||||
|
||||
Validation:
|
||||
|
||||
- Destroy a loader immediately after queuing several tasks.
|
||||
- Verify under ASAN that no task or callback accesses destroyed loader members.
|
||||
- Run synchronization coverage under TSAN where available.
|
||||
|
||||
Work-package exit criteria:
|
||||
|
||||
- TextureAtlasLoader's required destruction order is documented and covered by a regression test.
|
||||
- ResourceLoader status/progress reads are data-race-free.
|
||||
- No callbacks execute under internal synchronization locks.
|
||||
|
||||
Status: implemented. Status/progress counters are atomic, task and callback container access is
|
||||
synchronized, and callbacks execute after releasing internal locks. `ResourceLoader` joins its
|
||||
worker in its destructor without exposing a terminal shutdown API. `TextureAtlasLoader` declares
|
||||
`mRL` last, documents the order invariant, and publishes asynchronous status through atomics.
|
||||
Focused ASAN coverage verifies atlas destruction while tasks and a completion callback are pending.
|
||||
All six `ResourcePrerequisites` tests pass under both ASAN and unsuppressed TSAN.
|
||||
|
||||
## 5. Work package 3: shared ThreadPool HTTP operation lifetime
|
||||
|
||||
This follows the already fixed `Http::Pool::clear()` lock-order defect.
|
||||
|
||||
Current defect:
|
||||
|
||||
When `Http::setThreadPool()` is active, queued lambdas capture raw `Http*`. `Http` tracks and joins
|
||||
only its privately created `AsyncRequest` threads, so a shared-pool operation may begin or continue
|
||||
after its Http object has been destroyed.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Register every asynchronous operation before publishing it to an executor.
|
||||
- Give queued/running operations lifetime and cancellation state independent of raw `Http*`.
|
||||
- `Http::~Http()` rejects new work, cancels all registered operations, and waits until no operation
|
||||
can dereference the object.
|
||||
- Handle destruction initiated from an operation callback without joining/waiting on the same
|
||||
operation.
|
||||
- Do not destroy or drain an externally owned shared ThreadPool.
|
||||
- Do not wait while holding the global Http Pool mutex, operation-map mutex, or any lock visible to
|
||||
callbacks.
|
||||
- Preserve cancellation callback behavior deliberately and document it.
|
||||
- Cover all three async forms: response in memory, external IOStream, and output path.
|
||||
|
||||
Validation:
|
||||
|
||||
- Queue a request behind blocked shared-pool work, destroy/clear its Http owner, then unblock it.
|
||||
- Destroy while a request is running.
|
||||
- Re-enter `Http::Pool` from a callback during concurrent pool clearing.
|
||||
- Initiate final-owner release from a callback and verify no self-deadlock.
|
||||
- Run under ASAN and TSAN.
|
||||
|
||||
Work-package exit criteria:
|
||||
|
||||
- No shared-pool lambda depends on an untracked raw Http lifetime.
|
||||
- Http destruction provides a complete operation barrier without taking ownership of the executor.
|
||||
|
||||
Status: implemented. Shared-pool operations are heap-backed, registered before executor
|
||||
submission, and unregister from their destructor even when queued work is discarded. Pool-managed
|
||||
Http instances remain alive through callback completion, while stack/raw
|
||||
instances use the destructor barrier. Pool clearing requests cancellation and waits outside the
|
||||
Pool mutex. When clearing is initiated by a shared-pool callback, it defers the shared-operation
|
||||
barrier because queued work may require the current executor thread; shared operations retain the
|
||||
clients until that work drains. Cancellation state is atomic and shared by request copies. Focused
|
||||
ASAN coverage for all three async output forms, running cancellation, and callback-initiated
|
||||
clearing with queued work passes. All six focused HTTP tests also pass under unsuppressed TSAN.
|
||||
|
||||
## 6. Work package 4: deterministic Engine shutdown
|
||||
|
||||
Reorder shutdown only after HTTP and loader barriers are reliable.
|
||||
|
||||
### 6.1 Required dependency order
|
||||
|
||||
The exact implementation may group calls differently, but it must preserve this dependency graph:
|
||||
|
||||
```text
|
||||
mark Engine shutting down / reject new deliveries
|
||||
-> stop and join HTTP and resource-producing work
|
||||
-> invalidate and purge UI main-thread deliveries
|
||||
-> destroy scenes
|
||||
-> flush or discard GlobalBatchRenderer submissions
|
||||
-> clear TextLayout and shaped-font caches
|
||||
-> destroy UI/global drawable providers and resource managers
|
||||
-> destroy fonts, atlases and textures in dependency order
|
||||
-> destroy shader programs and shaders
|
||||
-> destroy framebuffer and vertex-buffer registries/owners
|
||||
-> destroy Renderer
|
||||
-> destroy windows and GL contexts
|
||||
-> destroy process utilities and backend state
|
||||
```
|
||||
|
||||
Concrete corrections required:
|
||||
|
||||
- Stop `Network::Http::Pool` and Engine-owned resource producers before Graphics consumers.
|
||||
- Destroy `SceneManager` before `GlobalBatchRenderer` and `NinePatchManager` resources scenes can
|
||||
reference.
|
||||
- Flush or explicitly discard pending batches before releasing their borrowed dependencies.
|
||||
- Call `TextLayout::clearLayoutCache()` before `FontManager::destroySingleton()`.
|
||||
- Destroy `ShaderProgramManager` before `Renderer` while `GLi` and a valid context still exist.
|
||||
- Keep windows/context state alive through every GPU-object destruction step.
|
||||
|
||||
### 6.2 Validation
|
||||
|
||||
- Engine destruction with a live scene using fonts, nine-patches, textures, batches, shaders, FBOs,
|
||||
and vertex buffers.
|
||||
- Engine destruction with pending HTTP and decode/resource-loader work.
|
||||
- Multiple Engine create/destroy cycles in one test process.
|
||||
- Assert no destructor recreates an Engine or manager singleton.
|
||||
- ASAN/LSAN clean teardown in supported configurations.
|
||||
|
||||
Work-package exit criteria:
|
||||
|
||||
- All asynchronous producers are behind a shutdown barrier before their consumers are destroyed.
|
||||
- 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 Package 5.
|
||||
The complete ASAN unit suite passes: 749 tests passed and one opt-in visual test was skipped.
|
||||
|
||||
## 7. Work package 5: UISceneNode async delivery lifecycle
|
||||
|
||||
Current behavior:
|
||||
|
||||
Async resource deliveries are stored in a process-static queue. Generation/alive checks prevent
|
||||
many stale callbacks from mutating a dead scene, but queued closures and their captures can survive
|
||||
until an unrelated future scene update and cross an Engine test boundary.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Add explicit accept/reject/drain/purge lifecycle operations for the queue.
|
||||
- Reject new deliveries once UI/Engine shutdown begins.
|
||||
- Invalidate scene generation/alive state before purging queued closures.
|
||||
- Release captures on the main/update thread, following the existing project destruction contract.
|
||||
- Re-open/reset the delivery mechanism deliberately for a recreated test Engine.
|
||||
- Do not allow work queued by Engine lifecycle A to execute during lifecycle B.
|
||||
|
||||
Validation:
|
||||
|
||||
- Queue immediate and delayed deliveries, destroy the scene before update, and verify neither runs.
|
||||
- Destroy and recreate Engine, then update a new UISceneNode and verify no old closure executes.
|
||||
- Race worker submission with queue shutdown under TSAN.
|
||||
|
||||
Work-package exit criteria:
|
||||
|
||||
- The static queue has an explicit Engine lifecycle boundary.
|
||||
- Purging releases all stale captures deterministically.
|
||||
|
||||
## 8. Work package 6: FrameBufferFBO recreation correctness
|
||||
|
||||
First determine the intended callers and semantics of `FrameBufferFBO::reload()`.
|
||||
|
||||
Required distinction:
|
||||
|
||||
- Live-context recreation must release the previous framebuffer/renderbuffer objects before
|
||||
replacing their handles.
|
||||
- Context-loss recreation must forget names from the lost namespace without issuing invalid delete
|
||||
calls against the replacement context.
|
||||
|
||||
Additional create-path audit:
|
||||
|
||||
- Restore prior framebuffer/renderbuffer bindings on every failure return.
|
||||
- Release partially created objects on live-context failure.
|
||||
- Leave the object in a destructible, clearly invalid state after failure.
|
||||
- Do not change texture-attachment ownership in this package; that belongs to TexturePtr Stage 2.
|
||||
|
||||
Validation:
|
||||
|
||||
- Repeated live-context recreation does not grow tracked GL object counts.
|
||||
- Context-loss recreation performs no deletion in the lost namespace.
|
||||
- Forced create failures restore previous bindings and do not leak partial objects.
|
||||
|
||||
Work-package exit criteria:
|
||||
|
||||
- `reload()` has an explicit live/lost-context contract.
|
||||
- Replacement and failure paths have deterministic GL-handle cleanup.
|
||||
|
||||
## 9. Work package 7: TextureLoader callback registry safety
|
||||
|
||||
Current behavior:
|
||||
|
||||
`TextureLoader::sCbs` is process-static and unsynchronized. Loading may notify from a worker while
|
||||
UITextureViewer or another caller registers/removes callbacks.
|
||||
|
||||
Fix requirements:
|
||||
|
||||
- Synchronize callback registration and removal.
|
||||
- Copy/snapshot callbacks under the lock and invoke the snapshot after unlocking.
|
||||
- Define removal-during-notification behavior.
|
||||
- Add an explicit test/Engine lifecycle reset operation if the registry remains process-static.
|
||||
- Ensure callbacks from an old Engine lifecycle cannot target UI state in a recreated Engine.
|
||||
|
||||
Validation:
|
||||
|
||||
- Concurrent registration, removal, and notification under TSAN.
|
||||
- Callback re-entry into registration/removal does not deadlock.
|
||||
- Repeated Engine lifecycle does not inherit callback subscriptions.
|
||||
|
||||
This package may be omitted only if Stage 1 immediately replaces this registry with the accepted
|
||||
weak live-texture diagnostics mechanism. The omission must be an explicit Stage 1 scope decision,
|
||||
not an assumption.
|
||||
|
||||
## 10. Deferred findings: do not solve in this plan
|
||||
|
||||
The following defects remain recorded but should normally be resolved by their owning migration
|
||||
stage because a raw-pointer workaround would be short-lived or semantically incomplete:
|
||||
|
||||
- `Variant` copying an owning Drawable pointer: resolve with the Stage 4 handle/`std::variant`
|
||||
redesign unless a current reproducer requires an emergency restriction.
|
||||
- `UISkin::clone()` and `StateListDrawable` duplicating ownership maps: resolve with Stage 4
|
||||
source/instance semantics unless a current owning-child clone path is demonstrated.
|
||||
- FrameBuffer texture attachment direct/factory ownership: resolve in the complete TexturePtr
|
||||
holder cut in Stage 2.
|
||||
- TextureRegion, TextureAtlas, fonts, glyphs, sprites, particles, and batches borrowing textures:
|
||||
resolve together in Stage 2.
|
||||
- Mutable Drawable sharing and incorrect `isStateful()` classifications: resolve in Stage 4.
|
||||
- TextureFactory lock scope, weak diagnostics, semantic lookup, metrics, and deferred release:
|
||||
these are Stage 1 through Stage 3 architecture work, not prerequisite patches.
|
||||
|
||||
## 11. Final prerequisite gate
|
||||
|
||||
Stage 1 may begin when all mandatory packages satisfy these invariants:
|
||||
|
||||
- HTTP and ResourceLoader work cannot outlive the objects they dereference.
|
||||
- Engine shutdown rejects and joins asynchronous resource producers before destroying scenes or
|
||||
Graphics systems.
|
||||
- UISceneNode queued delivery cannot cross an Engine lifecycle boundary.
|
||||
- Scenes and caches are destroyed before the resources they borrow.
|
||||
- Shader, font-layout, Renderer, and context destruction order is valid.
|
||||
- TextureAtlasLoader destruction is safe with active work.
|
||||
- FrameBuffer recreation cannot leak or delete objects from the wrong context namespace.
|
||||
- Focused ASAN tests and repeated Engine create/destroy tests pass.
|
||||
|
||||
After this gate, start Stage 1 immediately. Any newly discovered issue is added to this prerequisite
|
||||
track only if it violates one of these invariants; otherwise it is assigned to its resource-family
|
||||
migration stage.
|
||||
@@ -1,17 +1,11 @@
|
||||
# eepp shared-resource ownership architecture
|
||||
|
||||
Status: architecture baseline, revised after lifetime-contract review, 2026-07-12.
|
||||
Status: active implementation baseline; Stage 0 and prerequisite fixes complete; Stage 1 is next,
|
||||
2026-07-14.
|
||||
|
||||
Stage 0 inventory and shutdown dependency graph:
|
||||
`resource_shared_ownership_stage0_inventory.md`.
|
||||
|
||||
Prerequisite defect track:
|
||||
`resource_refactor_prerequisite_bugfixes.md`.
|
||||
|
||||
This document supersedes `resource_shared_ownership_refactor_plan.md`. It incorporates the review
|
||||
of that draft and freezes the contracts that must be true before the public texture API is changed.
|
||||
The implementation may refine names and small mechanics, but changing an invariant below requires
|
||||
an explicit architecture revision.
|
||||
This document freezes the contracts that must be true before the public texture API is changed. The
|
||||
implementation may refine names and small mechanics, but changing an invariant below requires an
|
||||
explicit architecture revision.
|
||||
|
||||
## 1. Objective
|
||||
|
||||
@@ -21,7 +15,7 @@ explicit shared ownership throughout eepp and all in-repository consumers.
|
||||
The final model is:
|
||||
|
||||
- Consumers, immutable source objects, catalogs, and caches own resources with strong handles.
|
||||
- A live registry observes resources weakly for diagnostics, accounting, context recovery, and leak
|
||||
- A live registry observes resources weakly for diagnostics, accounting, and leak
|
||||
reporting. It is never searched for semantic names.
|
||||
- Catalogs define names and persistence.
|
||||
- Scopes define which catalogs and typed caches are visible.
|
||||
@@ -157,7 +151,7 @@ UI/Network::WebResourceCache
|
||||
```
|
||||
|
||||
Engine coordinates the Graphics lifetime roots directly. TextureFactory coordinates texture
|
||||
creation, weak observation, reload and deferred destruction, but does not provide semantic lookup or
|
||||
creation, weak observation, and deferred destruction, but does not provide semantic lookup or
|
||||
normal strong retention. No Graphics class depends on UI.
|
||||
|
||||
### 4.1 LiveResourceRegistry
|
||||
@@ -189,9 +183,7 @@ void purgeExpired();
|
||||
Opening `UITextureViewer` must not retain all textures. The viewer may lock a weak handle for one
|
||||
render operation and may strongly retain only a user-selected texture.
|
||||
|
||||
For context loss/reload, the registry locks live weak handles into a temporary strong vector while
|
||||
holding its mutex, releases the mutex, and then performs device work. Destruction, callbacks, and GL
|
||||
operations never occur while a registry lock is held.
|
||||
Destruction, callbacks, and GL operations never occur while a registry lock is held.
|
||||
|
||||
### 4.2 ResourceMetrics
|
||||
|
||||
@@ -509,7 +501,7 @@ the feature branch to keep behavior valid while the repository-wide API break is
|
||||
|
||||
### Stage 0: contract freeze and inventories
|
||||
|
||||
Status: complete. See `resource_shared_ownership_stage0_inventory.md`.
|
||||
Status: complete. Its accepted conclusions are incorporated into this architecture baseline.
|
||||
|
||||
Deliverables:
|
||||
|
||||
@@ -527,8 +519,8 @@ or drawable-sharing contract blocks substrate implementation.
|
||||
|
||||
### Stage 0.5: prerequisite bug fixes
|
||||
|
||||
Land defects discovered by the ownership audit independently of the API refactor. Each fix receives
|
||||
focused regression coverage and preserves current raw factory ownership. Initial set:
|
||||
Status: complete, 2026-07-14. Concrete defects discovered by the ownership audit were fixed with
|
||||
focused regression coverage while preserving current raw factory ownership:
|
||||
|
||||
- HTTP Pool clears clients outside its mutex so joined callbacks can re-enter without deadlock.
|
||||
- Externally executed HTTP tasks cannot retain a dangling raw Http after Pool destruction.
|
||||
@@ -537,9 +529,10 @@ focused regression coverage and preserves current raw factory ownership. Initial
|
||||
- Engine destroys ShaderProgramManager before Renderer and clears TextLayout before FontManager.
|
||||
- Engine stops asynchronous resource producers before resource consumers and GPU managers.
|
||||
- UISceneNode's static async delivery queue has an explicit shutdown purge/rejection boundary.
|
||||
- TextureLoader's process-static callback registry receives a synchronization/reset contract.
|
||||
- Models::Variant drawable copying and UISkin/StateListDrawable shallow ownership bugs receive
|
||||
immediate containment or regression tests before their structural Stage 4 replacement.
|
||||
- Obsolete FrameBuffer context-loss reload APIs were removed.
|
||||
|
||||
TextureLoader's static callback registry is deliberately removed with Stage 1 live observation.
|
||||
Drawable ownership defects remain assigned to their structural Stage 4 replacement.
|
||||
|
||||
### Stage 1: texture lifetime scaffolding, with old factory retention still active
|
||||
|
||||
@@ -549,6 +542,9 @@ and graphics-thread assertions. Integrate collection into Window::display() afte
|
||||
into Engine shutdown before Renderer/context destruction. Reorder Engine teardown using the audited
|
||||
dependency graph. Do not generalize this substrate to self-contained GPU resource classes.
|
||||
|
||||
Remove TextureLoader's static callback registry in the same change and migrate UITextureViewer to
|
||||
the weak live registry. Do not add an intermediate synchronization/reset contract to the old API.
|
||||
|
||||
The old raw factory ownership remains temporarily so this internal stage cannot make resources
|
||||
disappear. Public texture APIs have not switched yet; the deferred shared-pointer deleter becomes
|
||||
active in the complete Stage 2 TexturePtr cut.
|
||||
@@ -580,7 +576,7 @@ At minimum migrate:
|
||||
- SVG raster caches and UI icons
|
||||
- sprites and particle systems that retain textures/regions
|
||||
- UIImage and UINodeDrawable texture paths
|
||||
- context reload and debug texture viewer
|
||||
- debug texture viewer and live-resource diagnostics
|
||||
- eepp, ecode, modules, examples, tools, and tests
|
||||
|
||||
Every stored raw Texture pointer is classified as strong, weak, or a short borrow dominated by a
|
||||
@@ -591,7 +587,6 @@ Exit criteria:
|
||||
- Regions, atlases, framebuffers, fonts, glyphs, UI consumers, and async work retain dependencies.
|
||||
- No ignored factory load result is relied upon for later global lookup.
|
||||
- No public texture delete/remove API remains.
|
||||
- Live registry reload sees externally owned textures.
|
||||
- Diagnostic snapshots do not pin resources.
|
||||
- Temporary factory retention can be removed without failing ownership tests.
|
||||
|
||||
@@ -676,7 +671,6 @@ Remove raw-owning `ResourceManager<T>` only when no subclass or consumer depends
|
||||
- Catalog publication retains; erasing one catalog entry does not invalidate other owners.
|
||||
- Independent catalog/pin leases do not interfere.
|
||||
- Registry snapshot and UITextureViewer do not retain all resources.
|
||||
- Context reload includes resources owned only by external handles.
|
||||
- Registry creation, snapshot, expiration, and purging are race-safe.
|
||||
|
||||
### GPU/thread lifetime
|
||||
@@ -686,7 +680,6 @@ Remove raw-owning `ResourceManager<T>` only when no subclass or consumer depends
|
||||
- Display flushes batches before collecting released textures under the current context.
|
||||
- Engine shutdown performs a final collection before TextureFactory/Renderer/context destruction.
|
||||
- No deletion/callback occurs while registry/cache locks are held.
|
||||
- Context loss/reload includes resources owned only by external handles.
|
||||
- Repeated test-only Engine creation starts with no prior texture handles or registry state.
|
||||
- Relevant suites run under TSAN as well as ASAN/LSAN.
|
||||
|
||||
@@ -731,9 +724,10 @@ Remove raw-owning `ResourceManager<T>` only when no subclass or consumer depends
|
||||
|
||||
## 12. First implementation deliverable
|
||||
|
||||
The next coding deliverable is Stage 0.5 prerequisite bug fixing. The Stage 0 inventories and
|
||||
shutdown dependency graph are complete and linked above. Bug fixes land with focused tests while
|
||||
preserving current public APIs and factory ownership.
|
||||
The next coding deliverable is Stage 1 TextureFactory-specific lifetime scaffolding. It adds weak
|
||||
live-texture observation and deferred texture collection while preserving current factory retention,
|
||||
removes TextureLoader's static callback registry, and migrates UITextureViewer to the live registry.
|
||||
|
||||
Only after those fixes are isolated should Stage 1 add TextureFactory-specific lifetime scaffolding.
|
||||
Stage 2 then changes public texture APIs and migrates all holders in one cut.
|
||||
Stage 1 must also audit TextureFactory's uncalled context-recovery-era reload/grab/ungrab APIs and
|
||||
remove them if repository and history inspection confirm they are obsolete. Stage 2 then changes
|
||||
public texture APIs and migrates all holders in one cut.
|
||||
|
||||
@@ -1,538 +0,0 @@
|
||||
# Shared-resource refactor Stage 0 inventory
|
||||
|
||||
Status: complete repository audit, lifetime contract revised 2026-07-12.
|
||||
|
||||
This document is the evidence and dependency inventory required by Stage 0 of
|
||||
`resource_shared_ownership_architecture.md`. Searches covered `include/`, `src/eepp/`, in-tree
|
||||
modules, tools, examples, ecode, and tests. Third-party implementation directories were excluded
|
||||
except where eepp invokes their APIs.
|
||||
|
||||
Confirmed defects are tracked for implementation in
|
||||
`resource_refactor_prerequisite_bugfixes.md`.
|
||||
|
||||
The inventory classifies stored relationships, side-effect loads, explicit deletion, callbacks,
|
||||
GPU object namespaces, drawable mutation, asynchronous producers, and Engine teardown dependencies.
|
||||
Line numbers will drift; paths and symbols are the stable references.
|
||||
|
||||
## 1. Stage 0 conclusions
|
||||
|
||||
The architecture contracts can proceed with these refinements:
|
||||
|
||||
1. eepp retains its existing graphics-thread-affine project contract. Shared ownership does not
|
||||
promise arbitrary-thread final destruction. Texture is deferred through TextureFactory and
|
||||
collected from `Window::display()` after batch flush; other self-contained GPU objects retain
|
||||
their direct graphics-thread destruction model.
|
||||
2. Every queued renderer submission owns its dependencies. `BatchRenderer` currently stores a raw
|
||||
texture between `setTexture()` and a later `flush()`; the migrated batch must retain a
|
||||
`TexturePtr` until flushing or discarding its vertices.
|
||||
3. Text layout is an owner/cache boundary. The global `TextLayout` LRU stores layouts containing
|
||||
`ShapedGlyph::font` raw pointers. In the final design layouts retain the required `FontPtr`
|
||||
values; the bounded global LRU is then an intentional cache owner and is explicitly clearable for
|
||||
test isolation.
|
||||
4. Resource-producing work must have an explicit cancellation/lifetime owner independently of an
|
||||
arbitrary shared `ThreadPool`. UISceneNode pools can be shared with a host or application and
|
||||
cannot simply be destroyed by Engine. WebResourceCache and the owning scene/document services
|
||||
track their own operations and subscribers; no generic Graphics ResourceSystem is required.
|
||||
5. Existing shutdown functions cannot only be reordered:
|
||||
- `Http::Pool::clear()` clears clients while holding the pool mutex; each client joins request
|
||||
callbacks, so a callback re-entering the global pool can deadlock.
|
||||
- When `Http::setThreadPool()` is active, queued lambdas capture raw `Http*`. `Http::~Http()`
|
||||
cancels requests but only joins privately created request threads; it does not join shared-pool
|
||||
work. Clearing the Pool can therefore destroy Http while a shared-pool task still uses it.
|
||||
- `TextureAtlasLoader` declares `ResourceLoader mRL` before the state used by its callbacks.
|
||||
Members are destroyed in reverse declaration order, so the loader is destroyed last and can
|
||||
run against already-destroyed state.
|
||||
These are prerequisite bugs and should be fixed independently before the ownership refactor.
|
||||
|
||||
Status: the HTTP Pool lock-order defect and shared-ThreadPool operation lifetime defect are
|
||||
fixed. Pool clearing now establishes an operation barrier without owning the executor, including
|
||||
callback-initiated clearing and executor-discarded queued work. The TextureAtlasLoader lifetime
|
||||
defect is also fixed and covered by sanitizer-backed regression tests.
|
||||
6. `isStateful()` is unusable as a shareability test. Every current Drawable inherits mutable color
|
||||
and position, and several classes reporting false mutate themselves or children during draw.
|
||||
7. Stage 2 must migrate texture holders, loaders, ID-based construction, and queued batches in one
|
||||
cut. Factory-wide temporary retention may then be removed only in Stage 3 after catalogs/scopes
|
||||
replace global semantic lookup.
|
||||
|
||||
No unresolved architectural choice remains in Stage 0. The confirmed defects are tracked as Stage
|
||||
0.5 bug fixes before texture lifetime scaffolding begins.
|
||||
|
||||
## 2. GPU and device-affinity inventory
|
||||
|
||||
### 2.1 Context topology
|
||||
|
||||
`Engine` owns a map of `Window*` and tracks one current window. SDL2 and SDL3 windows each own a
|
||||
primary GL context and may own a second worker context when `SharedGLContext` is enabled:
|
||||
|
||||
- `src/eepp/window/engine.cpp`: `createWindow()`, `setCurrentWindow()`, `mWindows`.
|
||||
- `src/eepp/window/backend/SDL2/windowsdl2.cpp`: `mGLContext`, `mGLContextThread`,
|
||||
`setGLContextThread()`.
|
||||
- `src/eepp/window/backend/SDL3/windowsdl3.cpp`: equivalent context pair.
|
||||
- `Texture` and `TextureLoader` currently acquire the current window's worker context directly.
|
||||
|
||||
The refactor needs a process-unique `ResourceId` for diagnostics. It does not introduce public
|
||||
device epochs or share-group disposal identities. Existing context-current/shared-worker rules
|
||||
remain authoritative. Tests may destroy and recreate the singleton Engine, but they must first
|
||||
release all GPU resource handles and verify that factory/manager state is empty.
|
||||
|
||||
### 2.2 Device-affine object table
|
||||
|
||||
| Object/payload | Current creation and mutation | Current destruction | Current tracker/owner | Required migration |
|
||||
|---|---|---|---|---|
|
||||
| Texture GL handle | SOIL and texture upload paths in `texture.cpp` and `textureloader.cpp`; lock, unlock, replace, resize, reload and filter operations issue GL | `Texture::~Texture()` calls `glDeleteTextures` and changes worker context | TextureFactory owns raw Texture and binding/memory state | Final TexturePtr release queues the Texture in TextureFactory; `Window::display()` flushes batches then collects it under the current context; shutdown performs a final collection |
|
||||
| Temporary readback FBO | GLES `Texture::iLock()` creates, attaches, reads and deletes a framebuffer | Deleted synchronously inside `iLock()` | Stack-local handle | Keep as a device-thread scoped command; never execute from arbitrary last-release thread |
|
||||
| FrameBuffer FBO | `FrameBufferFBO::create()/resize()/reload()` creates framebuffer and depth/stencil/color renderbuffers | `FrameBufferFBO::~FrameBufferFBO()` directly deletes renderbuffers/FBO and may unbind | FrameBuffer self-registers in non-owning FrameBufferManager; SceneNode/UIWindow/TerminalDisplay own raw objects | Preserve graphics-thread destruction; migrate attachment ownership without a generic GPU disposal layer |
|
||||
| FrameBuffer texture attachment | `FrameBufferFBO::create()` asks TextureFactory for empty texture | `FrameBuffer::~FrameBuffer()` directly deletes Texture | FrameBuffer exclusive raw ownership, factory also believes it owns the same texture | FrameBuffer stores TexturePtr; no direct deletion; factory registry remains weak |
|
||||
| VBO/EBO/VAO | `VertexBufferVBO` creates/updates buffers and VAO | `VertexBufferVBO::clear()` directly deletes buffers/VAO; destructor calls clear | VertexBuffer self-registers in non-owning VertexBufferManager; consumer owns raw object | Preserve graphics-thread destruction; owning consumer eventually uses a handle or value owner |
|
||||
| Renderer streaming VBO/VAO | `RendererGL3CP` owns eight VBOs and one VAO | `RendererGL3CP::~RendererGL3CP()` directly deletes them | Renderer | Preserve Renderer-owned direct destruction before context teardown |
|
||||
| Shader object | `Shader::Init()/reload()` calls `GLi->createShader`, compiles source | `Shader::~Shader()` directly calls `GLi->deleteShader` | ShaderProgram manually owns raw Shader children | Preserve graphics-thread destruction; ShaderProgram eventually owns ShaderPtr/source handles |
|
||||
| Program object | `ShaderProgram::init()/reload()` creates and links program | Destructor directly calls `GLi->deleteProgram`, deletes Shader children, self-removes from manager | ShaderProgramManager raw-owns programs; Renderer stores raw default/current program pointers | Preserve graphics-thread destruction and ensure manager precedes Renderer; ownership becomes explicit later |
|
||||
| Primitive/UI geometry buffers | PrimitiveDrawable, UIBackgroundDrawable and UIBorderDrawable create VertexBuffer objects | Their destructors directly delete VertexBuffer | Per-drawable exclusive raw ownership | Per-consumer drawable owns VertexBuffer handle under the graphics-thread contract |
|
||||
| Terminal geometry/FBO | TerminalDisplay owns FrameBuffer, background/foreground VBs and style VB vector | Explicit delete/recreate paths | TerminalDisplay | Strong handles; release before device gate closes |
|
||||
|
||||
Direct deletion sites found by the audit:
|
||||
|
||||
- `src/eepp/graphics/texture.cpp`: texture and temporary framebuffer deletion.
|
||||
- `src/eepp/graphics/framebufferfbo.cpp`: framebuffer/renderbuffer deletion.
|
||||
- `src/eepp/graphics/vertexbuffervbo.cpp`: buffer and vertex-array deletion.
|
||||
- `src/eepp/graphics/renderer/renderergl3cp.cpp`: renderer VBO/VAO deletion.
|
||||
- `src/eepp/graphics/shader.cpp`: shader deletion.
|
||||
- `src/eepp/graphics/shaderprogram.cpp`: program deletion.
|
||||
|
||||
Renderer wrapper implementations in `src/eepp/graphics/renderer/renderer.cpp` expose the GL delete
|
||||
entry points; they are dispatch, not independent owners.
|
||||
|
||||
### 2.3 Non-owning GPU registries and consumers
|
||||
|
||||
- `FrameBufferManager : Container<FrameBuffer>` and `VertexBufferManager : Container<VertexBuffer>`
|
||||
observe raw self-registering objects and do not delete them.
|
||||
- FrameBuffer owners: SceneNode, UIWindow, TerminalDisplay, and direct application/test callers.
|
||||
- VertexBuffer owners: PrimitiveDrawable, UIBackgroundDrawable, UIBorderDrawable, TerminalDisplay,
|
||||
renderer internals, and direct application/test callers.
|
||||
- Renderer shader arrays (`RendererGL3`, `RendererGL3CP`, `RendererGLES2`) are raw views of programs
|
||||
currently owned by ShaderProgramManager.
|
||||
- `GlobalBatchRenderer` is CPU storage but retains a borrowed Texture pointer until a later flush.
|
||||
It is a real lifetime owner in the new model whenever `mNumVertex != 0`.
|
||||
|
||||
### 2.4 Device-operation rule
|
||||
|
||||
Creation, upload, mutation, context reload, final ownership release and deletion are graphics-
|
||||
affine. Async CPU decode may run elsewhere; GPU work uses the main context or an API that explicitly
|
||||
acquires the existing shared worker context. The refactor does not broaden this contract. Debug
|
||||
assertions and tests should detect wrong-thread release rather than adding generic device scheduling.
|
||||
|
||||
## 3. Texture ownership and lookup inventory
|
||||
|
||||
### 3.1 Persistent raw texture holders
|
||||
|
||||
| Holder | Field/API | Current lifetime assumption | Target classification |
|
||||
|---|---|---|---|
|
||||
| TextureFactory | `mTextures: id -> Texture*` | Sole global owner, semantic lookup, diagnostics and deletion | Weak LiveResourceRegistry records; no semantic lookup or ownership |
|
||||
| TextureLoader | `mTexture`, `getTexture()`, `OnTextureLoaded(Uint32, Texture*)` | Factory owns after loader returns | TexturePtr result/state/callback; operation owns during load |
|
||||
| TextureRegion | `mTexture`, ID constructors and `setTextureId()` | Factory keeps texture alive | Immutable region source stores TexturePtr; per-consumer region drawable stores source |
|
||||
| TextureAtlas | `mTextures` and ResourceManager-owned TextureRegion children | Factory owns textures; atlas owns regions | Atlas/source catalog owns TexturePtr and region-source handles |
|
||||
| TextureAtlasLoader | `mTexturesLoaded` plus ignored queued loads | Relies on factory side effects and later global name lookup | Operation retains TexturePtr results directly and passes them into atlas construction |
|
||||
| FrameBuffer | `mTexture` | FrameBuffer deletes attachment although factory also registers it | FrameBuffer stores TexturePtr |
|
||||
| FontTrueType::Page | `texture` and raw GlyphDrawable cache | Page explicitly removes texture from factory | Page stores TexturePtr; glyph source records retain page texture |
|
||||
| FontBMFont::Page | `texture` and raw GlyphDrawable cache | Same | Same |
|
||||
| FontSprite::Page | `texture` and raw GlyphDrawable cache | Same | Same |
|
||||
| GlyphDrawable | `mTexture` | Font page/factory assumed to outlive glyph | GlyphSource stores TexturePtr; render state is external/per consumer |
|
||||
| UISVGIcon | `mSVGs: size -> Texture*` | Factory owns raster cache | Icon/source cache stores TexturePtr with explicit cache policy |
|
||||
| ParticleSystem | `const Texture* mTexture` resolved by ID | Factory owns | ParticleSystem stores TexturePtr or immutable texture-source handle |
|
||||
| maps::TileMap | `mTileTex` | Factory owns generated/named blank-tile texture | TileMap stores TexturePtr |
|
||||
| BatchRenderer | `const Texture* mTexture` | Caller/resource survives until deferred flush | Strong TexturePtr while queued; reset on flush/discard |
|
||||
| Tests/test harness | vectors, arrays and locals in `src/tests/test_all` and unit tests | Factory teardown cleans up | Test-local handles/catalog fixtures |
|
||||
|
||||
Cursor APIs accept a Texture pointer but immediately lock and copy pixels into an Image. They are
|
||||
synchronous borrowed parameters, not persistent texture holders. They should accept `const
|
||||
TexturePtr&` or a documented borrowed `Texture&` depending on the final lock API.
|
||||
|
||||
UIColorPicker texture-returning helpers, image viewer, diff view, examples, and tool code mostly
|
||||
return/use local pointers but must receive TexturePtr because the result crosses a call boundary.
|
||||
|
||||
### 3.2 Texture-to-region-to-drawable chains
|
||||
|
||||
Raw texture lifetime is also hidden behind raw TextureRegion relationships:
|
||||
|
||||
- Sprite frame vectors store `TextureRegion*`, copy them shallowly, mutate frame region size/offset,
|
||||
and optionally delete regions/textures according to sprite flags.
|
||||
- NinePatch exclusively deletes nine generated TextureRegion children, each borrowing one Texture.
|
||||
- TextureAtlas raw-owns regions through ResourceManager.
|
||||
- GlobalTextureAtlas owns regions created by ID-based Sprite paths.
|
||||
- ScrollParallax, UITextureRegion, UISprite, maps GameObjectVirtual/GameObjectTextureRegion, map
|
||||
editor state, and UI editor image maps retain TextureRegion pointers.
|
||||
- TextureAtlasManager returns raw regions and vectors by name/pattern to Sprite and search callers.
|
||||
|
||||
Stage 2 therefore removes texture-ID construction and migrates all these region edges. Region IDs
|
||||
remain stable metadata if useful; they are not a lifetime acquisition mechanism.
|
||||
|
||||
### 3.3 Global semantic lookup sites
|
||||
|
||||
TextureFactory semantic lookup currently serves unrelated scopes:
|
||||
|
||||
- DrawableSearcher searches TextureAtlasManager, NinePatchManager and TextureFactory globally.
|
||||
- TextureAtlasLoader locates queued results by path after loading.
|
||||
- UIImage and UINodeDrawable use URL/path names as global cache keys.
|
||||
- Sprite, TextureRegion, NinePatch and ParticleSystem resolve numeric texture IDs.
|
||||
- Font page destructors remove by texture ID.
|
||||
- maps::TileMap resolves a generated blank-tile name.
|
||||
- ecode settings and uieditor resolve/remove application textures globally.
|
||||
- Tests rely on `getByName()`, `getTexture()`, and `getTextures()`.
|
||||
|
||||
All semantic name/path/URL acquisition moves to ResourceCatalog/ResourceScope. Numeric ResourceId
|
||||
lookup in LiveResourceRegistry is diagnostic/administrative and returns a weak handle; it is not a
|
||||
substitute for a catalog.
|
||||
|
||||
### 3.4 Loads relying on global side effects
|
||||
|
||||
Confirmed ignored or indirect load results:
|
||||
|
||||
- `TextureAtlasLoader` queued `loadFromPack()` and `loadFromFile()` calls; later lookup by path.
|
||||
- `src/tests/test_all/test.cpp` queues/discards pack loads and later resolves globally.
|
||||
- Theme directory loading directly embeds load results into new TextureRegion/NinePatch/Sprite
|
||||
graphs; those destination objects must retain handles in the same cut.
|
||||
- Font pages assign factory results to raw page fields and explicitly remove them later.
|
||||
- `Texture::loadGif()` returns a vector of raw frames; Sprite assumes ownership flags/global factory.
|
||||
|
||||
Other creation paths return a local pointer and immediately pass it to a current raw holder:
|
||||
|
||||
- FrameBufferFBO attachment creation.
|
||||
- UIImage/UINodeDrawable remote placeholders.
|
||||
- UISVGIcon and UISVG rasterization.
|
||||
- FontTrueType, FontBMFont and FontSprite page creation.
|
||||
- DrawableSearcher file/data/HTTP paths.
|
||||
- UIColorPicker, UIImageViewer, UIDiffView, uieditor and sprite examples.
|
||||
|
||||
The Stage 2 compile cut changes every one to retain or propagate TexturePtr. The temporary factory
|
||||
retention map is removed only after an audit asserts no ignored result is semantically required.
|
||||
|
||||
### 3.5 Explicit deletion and unload semantics
|
||||
|
||||
Current deletion paths that must disappear:
|
||||
|
||||
- `TextureFactory::remove(id)`, `remove(Texture*)`, `unloadTextures()` and `removeReference()`.
|
||||
- `TextureLoader::unload()`.
|
||||
- FontTrueType/FontBMFont/FontSprite Page destructors removing texture IDs.
|
||||
- FrameBuffer directly deleting its attachment.
|
||||
- uieditor explicitly removing textures loaded for its image map.
|
||||
- Sprite cleanup flags that can delete factory textures/regions.
|
||||
|
||||
Final equivalents are handle reset, catalog erase, cache eviction, and operation cancellation. None
|
||||
invalidates another consumer's resource.
|
||||
|
||||
### 3.6 Texture callbacks and global state
|
||||
|
||||
- TextureLoader has a process-static callback map with `Texture*` payloads. It is not synchronized,
|
||||
is not reset with Engine, and UITextureViewer is its only current subscriber.
|
||||
- DrawableResource Change/Unload callbacks use raw resource pointers and integer IDs.
|
||||
- UITextureViewer stores Texture pointer -> callback ID and expects Unload to remove rows.
|
||||
- TextureFactory memory accounting and Texture mutation call each other through the singleton.
|
||||
- TextureFactory performs reload/grab/ungrab while holding its registry lock and invokes texture/GL
|
||||
operations under that lock.
|
||||
|
||||
Target:
|
||||
|
||||
- LiveResourceRegistry emits diagnostic record changes or supplies snapshots; viewer uses weak
|
||||
handles.
|
||||
- Source mutation signals use RAII connections and weak subscriber tokens.
|
||||
- ResourceMetrics is captured state, independent of factory lifetime.
|
||||
- Registry locks protect records only; device work and callbacks occur after unlocking.
|
||||
|
||||
## 4. Drawable mutation and ownership inventory
|
||||
|
||||
### 4.1 Base-class result
|
||||
|
||||
`Drawable` itself stores mutable `mColor` and `mPosition`, and exposes setColor, setAlpha and
|
||||
setPosition. Consequently no existing subclass is generally shareable merely because its
|
||||
`isStateful()` returns false.
|
||||
|
||||
The target source/instance split remains valid, with one performance qualification: high-frequency
|
||||
glyph and text rendering should use immutable glyph sources plus external draw parameters instead of
|
||||
allocating a mutable drawable instance per rendered glyph.
|
||||
|
||||
### 4.2 Class classification
|
||||
|
||||
| Current class/family | Current mutation/ownership behavior | Target form |
|
||||
|---|---|---|
|
||||
| Texture | Mutable color/position from Drawable plus mutable GPU data, filters, clamp, local cache and name | Shared Texture resource only; drawing uses TextureDrawable instance or external draw params |
|
||||
| TextureRegion | Mutates destination size inside `draw(position,size)`; mutable source rect, offset, pixel cache, color/position | Immutable TextureRegionSource retaining TexturePtr; TextureRegionDrawable instance/presentation state |
|
||||
| GlyphDrawable | Cached and shared by font pages but has color, position, draw mode, italic flag, offset, size and advance; UICodeEditor temporarily changes draw mode | Immutable GlyphSource; text/editor pass draw mode, color, position and size as parameters |
|
||||
| NinePatch | Owns nine mutable regions; draw updates own size/position and every child; propagates color/alpha | Immutable NinePatchSource plus private per-consumer NinePatchDrawable layout state |
|
||||
| Sprite | Animation state, callbacks, transforms, current frame, repetitions and shallow-copied region vectors; mutates region size/offset | Per-consumer Sprite instance retaining immutable frame sources |
|
||||
| PrimitiveDrawable and Rectangle/Triangle/Arc/Circle/ConvexShape | Mutable geometry, color, position, fill/blend/line state and owned VertexBuffer cache | Per-consumer drawable instance; optional immutable geometry source only if later useful |
|
||||
| Linear/RadialGradientDrawable | Mutable stops, angle/shape/center/extent, size, color and position | Parsed immutable gradient source plus per-consumer instance, or fresh instance directly from CSS parser |
|
||||
| DrawableGroup | Optional global child-owner bool; draw/update mutates own size/position and child positions/alpha | Per-consumer composite owning private mutable child instances/source handles |
|
||||
| StateListDrawable | Raw state map plus pointer->bool ownership; mutable current state; draw temporarily changes child alpha; state color mutates child | Per-consumer state machine owning instances or source factories; no child mutation shared with another list |
|
||||
| UISkin | StateListDrawable; `clone()` shallow-copies pointers and ownership map, duplicating ownership claims | Skin source/definition in theme catalog; create independent skin/state-list instances |
|
||||
| RichText | Mutable layout/selection; stores raw inline/background/border drawables and temporarily recolors backgrounds during draw | Per-consumer RichText; retained source/instance handles; external color draw parameters |
|
||||
| UINodeDrawable | Node-owned layer map, geometry/cache state and nested background drawable | Per-node/per-consumer instance |
|
||||
| UINodeDrawable::LayerDrawable | Manual `mOwnsDrawable`; mutable repeat/clip/origin/size/offset; draw mutates child alpha/color; async placeholder | Per-node layer owning DrawablePtr instance created by UI::DrawableResolver |
|
||||
| UIBackgroundDrawable/UIBorderDrawable | Owner-node pointer, mutable geometry/radii/colors/position/size, owned VertexBuffer | Per-node instance; VertexBuffer handle |
|
||||
| DrawableResource/StatefulDrawable | Name/ID plus Change/Unload callback lifetime protocol | Source identity plus typed invalidation signal; no destructor Unload callbacks |
|
||||
|
||||
### 4.3 Other drawable holders requiring migration
|
||||
|
||||
- UIImage: raw drawable plus `mDrawableOwner`; temporarily recolors it during draw.
|
||||
- UIPushButton icon delegates the same ownership flag to UIImage.
|
||||
- UINode background/foreground APIs expose `ownIt`.
|
||||
- DrawableImageParser returns `Drawable*` plus `bool& ownIt` for gradients, shapes, URLs and icons.
|
||||
- UIIcon stores size -> Drawable raw pointers; UIGlyphIcon borrows FontTrueType and font-owned glyph
|
||||
drawables; UISVGIcon separately caches textures.
|
||||
- UITheme owns UISkin objects through ResourceManagerMulti; UIIconTheme manually owns UIIcon
|
||||
objects; skins point into global atlas/nine-patch resources.
|
||||
- Models::Variant stores Drawable in a C-style union and copies both pointer and owner flag.
|
||||
- RichText inline boxes/fragments store backgroundColorDrawable/backgroundDrawable/borderDrawable
|
||||
raw pointers.
|
||||
- UICodeEditor stores fold/unfold drawables and temporarily changes GlyphDrawable draw mode.
|
||||
- ClippingMask stores temporary borrowed Drawable pointers and calls draw later; its operation must
|
||||
remain bounded by the owner or retain instance handles while queued.
|
||||
- ecode/tool/plugin configuration and tab splitter structures store icon Drawable pointers.
|
||||
|
||||
### 4.4 Complete manual ownership-flag surface
|
||||
|
||||
The audit found ownership flags in:
|
||||
|
||||
- DrawableGroup (`mDrawableOwner`, `setDrawableOwner()`).
|
||||
- StateListDrawable (`mDrawablesOwnership`, per-state `ownIt`).
|
||||
- UISkin clone copying StateListDrawable ownership state.
|
||||
- UIImage (`mDrawableOwner`, `safeDeleteDrawable()`, `setDrawable(..., ownIt)`).
|
||||
- UINodeDrawable::LayerDrawable (`mOwnsDrawable`).
|
||||
- UINode background/foreground APIs.
|
||||
- UIPushButton icon API.
|
||||
- DrawableImageParser function and return protocol.
|
||||
- Models::Variant (`mOwnsObject` for Drawable).
|
||||
|
||||
All are removed in Stage 4. No compatibility overload remains.
|
||||
|
||||
## 5. Asynchronous producer inventory
|
||||
|
||||
| Producer | Work and current captures | Current stop behavior | Required contract |
|
||||
|---|---|---|---|
|
||||
| Http global Pool/Http AsyncRequest | UIWebView documents, UIImage/UINodeDrawable placeholders, DrawableSearcher, font faces; callbacks can mutate scenes/textures or queue main-thread work | Pool clear erases shared clients under mutex; Http destructor joins private request threads, but optional global-ThreadPool tasks capture raw Http and are not joined | Pool/service close rejects requests, swaps clients out under lock, unlocks, then cancels/joins all tracked operations regardless of executor; operation subscribers use weak session tokens |
|
||||
| UISceneNode ThreadPool | File texture load/upload, SVG/image decode, deferred fonts/styles; pool may be shared with host/application | Scene invalidates generation and deletes children; shared pool may outlive scene; ThreadPool destructor drains queued work by default | Owning scene/document service tracks operations, captures no raw scene, and is joinable; Engine does not destroy arbitrary external pools |
|
||||
| Static UISceneNode async-main queue | Lambdas queued by HTTP/thread workers with resource state/generation | Drained only during UISceneNode scheduled update; no Engine clear | UI-owned delivery queue has close/reject/invalidate/purge semantics before scene destruction |
|
||||
| TextureLoader | Decode and optional direct GL upload on calling/worker thread; static global callbacks | Stack loader; no service-level shutdown; callbacks unsynchronized | TextureLoadOperation owns TexturePtr/result; GPU upload follows explicit current/shared-context rules; typed observers are synchronized |
|
||||
| ResourceLoader | Internal Thread plus temporary ThreadPool drains all tasks before destructor returns | Destructor waits, but cannot cancel running work | Close/cancel token and join; callbacks never use destructed owner state |
|
||||
| TextureAtlasLoader | ResourceLoader tasks call factory, completion callback accesses loader and managers | ResourceLoader member is destroyed last due declaration order | Loader operation/state shared independently; join before state destruction; retain texture results directly |
|
||||
| UIWebView navigation | HTTP callbacks use weak NavigationLoadState and generation, then main-thread document replacement | Good stale-delivery guard, but request is global and cache-unaware | Per-document subscriber/session over shared request; stale tab does not cancel other subscribers |
|
||||
| UIImage/UINodeDrawable remote paths | Capture raw placeholder Texture and raw `this`, partially guarded by alive atomics/generation | Callback may still release/mutate on HTTP thread; factory owns placeholder | Capture TexturePtr and weak consumer token; decode off-thread, upload/device mutation scheduled, scene update generation-guarded |
|
||||
| UISVG and image tools | Shared scene pool tasks often capture raw `this` and later runOnMainThread | Per-widget tags/alive handling varies | Convert resource-producing paths to operation/subscriber tokens; application-only CPU tasks remain app responsibility |
|
||||
|
||||
ThreadPool itself waits for all queued work unless `terminateOnClose` is set. That behavior is useful
|
||||
but is not a global shutdown mechanism because ownership is distributed. Each UI/Web/cache service
|
||||
tracks operations that access its state even when execution uses a provided shared pool.
|
||||
|
||||
## 6. Current Engine teardown dependency audit
|
||||
|
||||
Current order in `Engine::~Engine()`:
|
||||
|
||||
```text
|
||||
GlobalBatchRenderer
|
||||
NinePatchManager
|
||||
SceneManager
|
||||
StyleSheetSpecification / SyntaxDefinitionManager
|
||||
FontManager
|
||||
TextureAtlasManager
|
||||
TextureFactory
|
||||
Renderer
|
||||
ShaderProgramManager
|
||||
PackManager
|
||||
FrameBufferManager / VertexBufferManager
|
||||
VFS
|
||||
SSL end
|
||||
HTTP Pool clear
|
||||
Windows / contexts
|
||||
backend and process caches
|
||||
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 |
|
||||
|---|---|
|
||||
| NinePatchManager before SceneManager | Scenes/widgets can still hold raw global nine-patch/region pointers; relies on Unload callbacks during teardown |
|
||||
| GlobalBatchRenderer destroyed first | Pending submissions are discarded without an explicit dependency release contract; future strong queued texture handles need explicit discard |
|
||||
| TextLayout cache after FontManager | Cached ShapedGlyph objects contain raw FontTrueType pointers after fonts are deleted |
|
||||
| Renderer before ShaderProgramManager | Renderer destruction sets global `GLi = nullptr`; ShaderProgram and Shader destructors then call through GLi |
|
||||
| TextureFactory before external FrameBuffer/VBO/program handles | External resources can outlive managers and their destructors can recreate manager/factory singletons or touch dead GL state |
|
||||
| FrameBufferManager/VertexBufferManager after Renderer | Managers are non-owning today, but any live registered object's deletion needs Renderer/context; order provides no guarantee |
|
||||
| HTTP Pool after all Graphics resources | Callbacks can mutate placeholders, create resources, enqueue scene work, or release final handles after consumers/device systems are gone |
|
||||
| HTTP Pool clear under its own mutex | Http destruction joins callbacks; callback re-entry to global Pool can deadlock |
|
||||
| HTTP using `sGlobalThreadPool` | Queued task captures raw Http; Pool clear can delete Http because its destructor cannot join externally executed work |
|
||||
| Windows last without per-context drain | Direct deletion happens against whichever context happens to be current, not necessarily the object's namespace |
|
||||
| Log before late static cache cleanup | Future abandoned-resource diagnostics would lose logging if any resource/cache remains |
|
||||
|
||||
## 7. Target shutdown dependency graph
|
||||
|
||||
### 7.1 Dependency graph
|
||||
|
||||
```text
|
||||
HTTP / decode / atlas / scene-pool executors
|
||||
│ produce
|
||||
▼
|
||||
Owning UI/document operation state + WebResourceCache requests
|
||||
│ deliver through generation/session tokens
|
||||
▼
|
||||
Scenes / UI documents / drawable instances / app caches
|
||||
│ retain
|
||||
├──────────────► Fonts ─► glyph sources ─► Textures
|
||||
├──────────────► Atlases ─► region sources ─► Textures
|
||||
├──────────────► Nine-patch/sprite sources ─► region sources
|
||||
├──────────────► FrameBuffers ─► attachment Textures
|
||||
└──────────────► VertexBuffers / queued batches
|
||||
|
||||
Global/default catalogs ───────────► any published resource/source
|
||||
TextLayout LRU ────────────────────► Fonts used by cached shaped runs
|
||||
Renderer ──────────────────────────► default Programs + streaming VBO/VAO
|
||||
|
||||
TexturePtr final release ─► TextureFactory released queue ─► Window::display collection
|
||||
Other GPU objects ────────────────────────────────────────► graphics-thread destruction
|
||||
Both paths ───────────────────────────────► Renderer/GL dispatch ─► Window context
|
||||
```
|
||||
|
||||
An arrow means the left side must stop producing or release its dependency before the right side is
|
||||
detached/destroyed. Shared handles make sibling release order less fragile, but graphics-thread and
|
||||
context order remain strict.
|
||||
|
||||
### 7.2 Concrete Engine shutdown sequence
|
||||
|
||||
1. **Enter shutdown.** Mark Engine, WebResourceCache and resource delivery queues as closing. Reject
|
||||
new resource, cache, HTTP-for-resource, upload, reload and main-thread delivery operations.
|
||||
2. **Invalidate subscribers.** Invalidate all document sessions, UISceneNode async generations,
|
||||
widget subscribers, navigation states, and cache leases. UI objects still exist, so cancellation
|
||||
callbacks that must observe them can do so through checked weak tokens.
|
||||
3. **Stop producers without holding service locks.** Swap global HTTP clients/request sets and
|
||||
tracked operations into local containers under their locks; unlock; cancel and join them. Close
|
||||
and join TextureAtlas/Texture load operations and Web cache fetch/decode operations. A provided
|
||||
external ThreadPool remains alive, but no tracked task may still access Engine/UI resource state
|
||||
after this barrier.
|
||||
4. **Purge delivery queues.** Remove pending UIScene/resource main-thread deliveries and release
|
||||
their captured handles. No queue can accept new entries after step 1.
|
||||
5. **Stop rendering submissions.** Discard pending GlobalBatchRenderer vertices and release its
|
||||
TexturePtr; ensure no window/scene draw is active. Do not attempt a cosmetic final render.
|
||||
6. **Destroy scenes/documents.** Destroy SceneManager and all child UISceneNodes/widgets. This
|
||||
releases document scopes, UI resolvers, UI themes/icons/skins, drawables, scene/app cache leases,
|
||||
SceneNode/UIWindow/Terminal framebuffers, primitive/UI/terminal vertex buffers, and scene-owned
|
||||
fonts. Scenes precede global source/catalog teardown.
|
||||
7. **Clear CPU caches retaining resources.** Clear TextLayout LRU and any font/glyph/drawable lookup
|
||||
caches. Clear application and global ResourceCatalogs/default scopes. Destroy StyleSheet and
|
||||
syntax/UI resolver specification state after scenes no longer use it.
|
||||
8. **Release high-level Graphics owners.** Release NinePatch, atlas/region, font/glyph and remaining
|
||||
theme/icon manager/catalog handles in dependency order. Clear font fallback/style links before
|
||||
releasing fonts. TextureFactory/LiveResourceRegistry remain present as weak observers only.
|
||||
9. **Collect released textures.** Make the active context current, flush/discard pending batch
|
||||
submissions, and call `TextureFactory::collectReleasedTextures()`. Purge expired weak records and
|
||||
report/assert any unexpected externally owned TexturePtr. Tests treat every survivor as a fixture
|
||||
teardown failure; a defensive GPU-payload release may make production shutdown safe to continue.
|
||||
10. **Release renderer-owned resources.** Destroy ShaderProgramManager and framebuffer/vertex
|
||||
owners before Renderer. Renderer then releases its default programs and streaming VBO/VAO while
|
||||
the context is valid. Assert TextureFactory has no pending released objects before it is destroyed.
|
||||
11. **Destroy Graphics roots.** Destroy TextureFactory's weak registry and CPU state, Renderer/GL
|
||||
dispatch, and non-owning manager shells. A TexturePtr surviving this point violates the project
|
||||
contract; it is not supported through a second device lifetime.
|
||||
12. **Destroy loading infrastructure.** Destroy PackManager and VFS after all tracked resource work
|
||||
and reload-capable internal owners are gone. End SSL after HTTP clients have joined.
|
||||
13. **Destroy windows and backend.** Destroy each worker/primary GL context and window, then platform,
|
||||
display and backend state.
|
||||
14. **Destroy process caches and logging last.** Clear SystemFontResolver/FreeType resolver state,
|
||||
parser/regex caches, and finally Log. MemoryManager reporting happens after test/application
|
||||
handles and catalog fixtures have been released.
|
||||
|
||||
### 7.3 Window destruction outside Engine teardown
|
||||
|
||||
`Engine::destroyWindow()` can remove one context while Engine and other windows continue. It needs
|
||||
the same per-context mini-sequence:
|
||||
|
||||
1. Reject new operations targeting that window/context.
|
||||
2. Cancel/join its tracked uploads and release window/scene-owned resources.
|
||||
3. Flush pending submissions and collect textures releasable under that current context.
|
||||
4. Release renderer/context-local payloads.
|
||||
5. Destroy the contexts/window.
|
||||
|
||||
The existing multi-window/shared-context contract determines which texture objects are valid under
|
||||
the remaining contexts. This refactor does not introduce a second context ownership model.
|
||||
|
||||
## 8. Test-isolation and build matrix decision
|
||||
|
||||
### 8.1 Required build configurations
|
||||
|
||||
The repository enables `EE_MEMORY_MANAGER` in debug configurations and builds both eepp static and
|
||||
shared libraries. Stage 1/2 validation therefore requires at least:
|
||||
|
||||
| Configuration | Purpose |
|
||||
|---|---|
|
||||
| Linux debug static, EE_MEMORY_MANAGER | Primary unit-test and tracked deleter correctness |
|
||||
| Linux debug shared, EE_MEMORY_MANAGER | ResourcePtr/custom-deleter behavior across library boundary |
|
||||
| Linux release static | Behavior without MemoryManager macros and optimized lifetime paths |
|
||||
| Linux release shared | Public handle ABI and cross-DSO destruction without debug tracking |
|
||||
| Debug static + AddressSanitizer/LeakSanitizer | UAF, double control block, leaks and late callbacks |
|
||||
| Debug static + ThreadSanitizer | Registry/cache/async operation races and contract violations |
|
||||
|
||||
Existing Premake options include `with-static-eepp`, `address-sanitizer`, and `thread-sanitizer`.
|
||||
Platform CI can expand after Linux substrate tests are stable; Windows/macOS context implementations
|
||||
must pass deferred texture collection and teardown-order tests before the public refactor is complete.
|
||||
|
||||
### 8.2 Per-test fixture contract
|
||||
|
||||
Each resource test owns an explicit fixture containing Engine/window if needed, catalogs/scopes, Web
|
||||
cache/session objects, and resource handles. Teardown order is:
|
||||
|
||||
1. invalidate subscribers and stop tracked operations;
|
||||
2. release test widgets/scenes/caches/catalogs/handles;
|
||||
3. flush batches, collect released textures and destroy Engine;
|
||||
4. purge expired registry records and static callbacks/caches;
|
||||
5. assert no unintended catalog entries, operation records, pending deliveries, released textures,
|
||||
or live texture registry entries;
|
||||
6. assert no resource handle survives Engine destruction;
|
||||
7. compare MemoryManager/registry diagnostics to fixture baseline.
|
||||
|
||||
No test may depend on a previous test's global TextureFactory name entry, TextureLoader callback,
|
||||
TextLayout cache, font fallback cache, or Engine ID counter reset.
|
||||
|
||||
## 9. Source-audit gates
|
||||
|
||||
Stage 2 and Stage 4 should preserve repeatable repository checks. The exact implementation may use
|
||||
clang-tidy, but these searches define the initial gates:
|
||||
|
||||
```sh
|
||||
rg '\b(?:const\s+)?Texture\s*\*' include src --glob '!src/thirdparty/**'
|
||||
rg 'TextureFactory::instance\(\)->(getByName|getByHash|getTexture|remove)' include src
|
||||
rg 'TextureFactory::instance\(\)->(loadFrom|createEmptyTexture|pushTexture)' include src
|
||||
rg '(ownIt|mOwnsDrawable|mDrawableOwner|mDrawablesOwnership|setDrawableOwner)' include src
|
||||
rg '(glDelete[A-Za-z]*|GLi->delete[A-Za-z]*)' include/eepp src/eepp
|
||||
```
|
||||
|
||||
The goal is not zero raw pointers everywhere. Allowed remaining matches must be one of:
|
||||
|
||||
- a local borrowed `.get()` view whose owning handle is in the same lexical object/call;
|
||||
- a synchronous reference parameter with documented lifetime;
|
||||
- low-level GL dispatch declarations;
|
||||
- a diagnostic weak-lock result used within the lock's strong-handle scope.
|
||||
|
||||
Every stored raw resource field requires an explicit code-review annotation and should normally be
|
||||
rejected.
|
||||
|
||||
## 10. Stage 0 exit assessment
|
||||
|
||||
Stage 0 deliverables are satisfied:
|
||||
|
||||
- GPU resource classes/direct deletion sites: inventoried.
|
||||
- Graphics-thread and TextureFactory deferred-release requirements: frozen.
|
||||
- Raw Texture holders, ID/name lookup, ignored loads, callbacks and deletion calls: inventoried.
|
||||
- Drawable classes, mutation and manual ownership surfaces: inventoried and classified.
|
||||
- Resource-related asynchronous producers and stop semantics: inventoried.
|
||||
- Current and target Engine shutdown dependency graph: documented.
|
||||
- Shared/static/MemoryManager/sanitizer build matrix: frozen.
|
||||
- Unit-test isolation contract: frozen.
|
||||
|
||||
Stage 0.5 fixes the concrete defects listed by this audit before ownership work resumes. Stage 1
|
||||
then adds TextureFactory-specific lifetime scaffolding without changing public Texture ownership;
|
||||
current factory retention remains active until the complete Stage 2 holder migration.
|
||||
Reference in New Issue
Block a user