From ab4bb0ddfacf03037fba3b16b125244617e63e6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Sun, 12 Jul 2026 20:50:39 -0300 Subject: [PATCH] Fix HTTP pool callback re-entry deadlock: Destroy pooled HTTP clients after releasing the pool mutex so joined callbacks can safely re-enter the global pool. Add a regression test covering concurrent pool clearing and callback re-entry. Document the revised texture lifetime architecture and track the prerequisite bugs discovered during the resource ownership audit. --- ...resource_refactor_prerequisite_bugfixes.md | 293 +++++++ .../resource_shared_ownership_architecture.md | 739 ++++++++++++++++++ ...ource_shared_ownership_stage0_inventory.md | 526 +++++++++++++ src/eepp/network/http.cpp | 10 +- src/tests/unit_tests/http.cpp | 90 +++ 5 files changed, 1656 insertions(+), 2 deletions(-) create mode 100644 .agent/plans/resource_refactor_prerequisite_bugfixes.md create mode 100644 .agent/plans/resource_shared_ownership_architecture.md create mode 100644 .agent/plans/resource_shared_ownership_stage0_inventory.md diff --git a/.agent/plans/resource_refactor_prerequisite_bugfixes.md b/.agent/plans/resource_refactor_prerequisite_bugfixes.md new file mode 100644 index 000000000..ec456a37a --- /dev/null +++ b/.agent/plans/resource_refactor_prerequisite_bugfixes.md @@ -0,0 +1,293 @@ +# Resource-refactor prerequisite bug fixes + +Status: active defect track, 2026-07-12. + +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. + +### 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. + +### 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. + +### 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. + +Fix: + +- Clear TextLayout and related shaped-font caches before FontManager. + +Regression coverage: + +- Populate shaped-layout cache, destroy Engine, and verify repeated Engine lifecycle under ASAN. + +### 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. + +## 4. Priority C: loader and callback lifetime + +### 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: + +- Add an explicit destructor shutdown/join before any callback-visible member is destroyed, or move + async operation state into a lifetime object that outlives execution. +- Do not rely only on member declaration order without an explicit invariant comment/test. + +Regression coverage: + +- Destroy a loader immediately with queued texture tasks and completion callbacks. + +### 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. diff --git a/.agent/plans/resource_shared_ownership_architecture.md b/.agent/plans/resource_shared_ownership_architecture.md new file mode 100644 index 000000000..5f61dc241 --- /dev/null +++ b/.agent/plans/resource_shared_ownership_architecture.md @@ -0,0 +1,739 @@ +# eepp shared-resource ownership architecture + +Status: architecture baseline, revised after lifetime-contract review, 2026-07-12. + +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. + +## 1. Objective + +Replace raw manager ownership, global load side effects, manual deletion, and `ownIt` flags with +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 + reporting. It is never searched for semantic names. +- Catalogs define names and persistence. +- Scopes define which catalogs and typed caches are visible. +- GPU resources remain graphics-thread-affine. The project contract requires final owning releases + and destruction to run through the graphics/update lifecycle rather than supporting arbitrary + last-release threads. +- UI drawable resolution is layered over Graphics resource lookup; browser caching and navigation + remain outside Graphics. +- A UISceneNode can own a scope and resolver, but neither texture lifetime nor pure Graphics usage + requires a UISceneNode. + +This is an intentional repository-wide API break. There will be no compatibility API, no `Shared` +suffixes, and no retained `ownIt` overloads. + +## 2. Confirmed hazards in the current code + +These are not hypothetical migration risks: + +- `TextureAtlasLoader` queues `TextureFactory::loadFromFile()` and `loadFromPack()` calls, discards + their results, then later resolves the textures globally by name. Immediately switching the + factory to weak, unpinned retention would compile and destroy each loaded texture at the end of + the lambda. +- `TextureLoader` stores and returns `Texture*`; its callback and unload behavior depend on the + factory owning the object. +- `TextureRegion` stores `Texture*`, and constructors taking a texture ID resolve that raw pointer + through the factory. +- `FrameBuffer` stores `Texture*` and deletes it directly in its destructor. +- `TextureAtlas`, glyph drawables, SVG icon raster caches, GIF loading, and font caches retain raw + texture pointers. +- `Texture::~Texture()` performs GL deletion and reaches `Engine::instance()` and + `TextureFactory::instance()`. Shared ownership must replace this singleton-dependent destruction + with TextureFactory-controlled deferred release on the graphics thread. +- `TextureRegion::isStateful()` and `DrawableGroup::isStateful()` return false despite draw/update + methods mutating destination size, position, or child state. +- `UIImage`, `StateListDrawable`, and `DrawableGroup` temporarily mutate drawable color, alpha, + size, position, or children while drawing. +- `Models::Variant` copies both an owning flag and the same raw drawable pointer, allowing two + copies to believe they exclusively own one allocation. +- `EE_MEMORY_MANAGER` requires allocation registration through `eeNew` and removal through + `eeDelete`; default `shared_ptr` deletion of an `eeNew` allocation would leave tracking invalid. +- `Engine::~Engine()` destroys `Renderer` before `ShaderProgramManager`, while + `ShaderProgram::~ShaderProgram()` calls `GLi->deleteProgram()` directly. +- The global HTTP pool is cleared after Graphics resources and managers, allowing asynchronous + producers to outlive systems they can mutate. + +## 3. Frozen ownership vocabulary + +### 3.1 Public handle representation + +eepp will expose `std::shared_ptr` and `std::weak_ptr` through consistent aliases: + +```cpp +template using ResourcePtr = std::shared_ptr; +template using ResourceWeakPtr = std::weak_ptr; + +using TexturePtr = ResourcePtr; +using TextureWeakPtr = ResourceWeakPtr; +``` + +This is an explicit API/ABI choice. eepp accepts that users can use standard shared-pointer +operations. The library nevertheless exposes no API for adopting an arbitrary resource raw pointer, +and resource constructors remain protected/private where practical. + +Requirements: + +- A resource allocation has exactly one control block. +- Owning APIs return handles. They do not return a raw pointer plus an ownership convention. +- Long-lived borrowed raw pointers are forbidden unless the field or API documents the owner that + dominates the borrow. Local `.get()` views inside a call are allowed. +- Shared-library and static-library builds must test the chosen handle/deleter behavior. + +### 3.2 Centralized creation and deletion + +All ref-counted eepp resource allocations go through one internal creation path with an +eepp-compatible deleter: + +```cpp +template struct ResourceDeleter { + void operator()( T* resource ) const noexcept { eeDelete( resource ); } +}; + +template +ResourcePtr makeResource( Args&&... args ); +``` + +Factories use an equivalent private helper for protected constructors. `std::make_shared` is not +used for tracked eepp resources unless the memory manager is redesigned to understand its combined +allocation. No second control block may be created from `handle.get()`. + +Texture is the deliberate exception to immediate `eeDelete`: its factory-controlled deleter queues +the final raw object for graphics-thread destruction. `TextureFactory::collectReleasedTextures()` +performs the eventual `eeDelete` after queued rendering has been flushed. This is the same deferred +destruction contract used by scene nodes; it is not a general arbitrary-thread GPU disposal system. + +### 3.3 Identity, keys, and labels + +These concepts are distinct: + +- `ResourceId` is immutable and process-unique across Engine recreation in tests. +- `ResourceKey` is the immutable canonical semantic lookup key. Equality compares the complete key, + never only a hash. +- `displayName` is diagnostic text and may change without changing identity or catalog indexes. +- Aliases are catalog entries, not mutable fields used as registry indexes. + +A process-wide monotonic ID source must not reset when an Engine singleton is recreated by tests. + +## 4. Graphics ownership layers + +```text +Engine +├── Renderer and contexts +├── TextureFactory +│ ├── weak live-texture registry +│ └── deferred released-texture queue +├── GlobalResourceCatalog (intentional strong persistence) +└── Default ResourceScope (local catalog + explicit imports) + +Application/scene ResourceScope +├── local ResourceCatalog +├── explicitly imported catalogs +└── typed caches + +UI::DrawableResolver +├── CSS/image/icon/glyph parsing +├── node and UISceneNode context +└── delegates texture/source lookup to Graphics::ResourceScope + +UI/Network::WebResourceCache +├── request/cache partitioning +├── in-flight request coalescing +├── TTL/LRU/byte-budget retention +└── per-document leases and subscribers +``` + +Engine coordinates the Graphics lifetime roots directly. TextureFactory coordinates texture +creation, weak observation, reload and deferred destruction, but does not provide semantic lookup or +normal strong retention. No Graphics class depends on UI. + +### 4.1 LiveResourceRegistry + +The registry observes every instantiated texture weakly: + +```cpp +struct TextureRecord { + ResourceId id; + ResourceKey creationKey; + std::string displayName; + TextureWeakPtr texture; + std::shared_ptr metrics; + ResourceFlags flags; +}; +``` + +It has no strong resource field, no `ownerScope`, no semantic name resolution, and no public +`unregister()` operation. + +Primary operations are snapshots, expiration purging, and live iteration. Diagnostic snapshots +return metadata plus weak handles, not a vector of owning texture handles: + +```cpp +TextureRegistrySnapshot snapshotTextures() const; +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. + +### 4.2 ResourceMetrics + +Texture memory accounting is held in shared record state captured at creation. Texture upload, +resize, replacement, and CPU-copy changes update atomics on that state directly; they never find a +factory singleton. Registry snapshots can read metrics even while expiration races with inspection. + +### 4.3 ResourceCatalog + +A catalog maps complete canonical keys/aliases to strong handles. It provides semantic lookup and +intentional persistence: + +```cpp +class ResourceCatalog { + public: + void publish( ResourceKey key, TexturePtr texture ); + TexturePtr findTexture( const ResourceKey& key ) const; + bool erase( const ResourceKey& key ); + void clear(); +}; +``` + +Publishing is the final global pin model. The low-level TextureFactory has no `ResourcePin` argument +and no factory-owned strong pin. Independent catalogs/caches naturally provide independent pins. +If temporary explicit pinning is needed, `ResourceCatalog::pin()` returns an independent RAII token; +there is no shared boolean or one global `strong` field. + +A higher-level scope convenience may accept a retention option and publish into that scope's catalog, +but texture creation and decoding remain unpinned operations. + +### 4.4 ResourceScope + +`Graphics::ResourceScope` performs Graphics-only lookup and loading: + +```cpp +class ResourceScope { + public: + TexturePtr findTexture( const ResourceKey& key ) const; + TexturePtr loadTexture( const TextureRequest& request ); + void publishLocal( ResourceKey key, TexturePtr texture ); + void importCatalog( ResourceCatalogPtr catalog ); +}; +``` + +Frozen lookup rules: + +- Search the local catalog, then explicitly imported catalogs in deterministic order. +- Never search the live registry. +- Never implicitly search a parent, host scene, sibling scene, or every live resource. +- The default Graphics scope imports the global catalog explicitly. +- A UI/application scene receives only the catalogs deliberately imported into it. +- A Web document does not inherit host/global resources unless the host exports and imports them + intentionally. +- Scopes import catalogs, not arbitrary scopes. This avoids recursive lookup and import cycles. + +Pure `EE::Graphics` users may use TextureFactory for unpinned creation or Engine's default Graphics +scope/catalog for named persistent resources. No UISceneNode is involved. + +### 4.5 TextureFactory final API role + +TextureFactory creates, decodes, uploads, and updates textures. Creation names return `TexturePtr` +directly and do not retain it: + +```cpp +TexturePtr createEmptyTexture( ... ); +TexturePtr loadFromPixels( ... ); +TexturePtr loadFromPack( ... ); +TexturePtr loadFromMemory( ... ); +TexturePtr loadFromStream( ... ); +TexturePtr loadFromFile( ... ); +``` + +The factory registers every result with its weak live registry. It has no semantic `getByName()` or +`getByHash()` API, no public deletion/removal API, no public registry detachment, and no owning +`getTextures()` API. Named lookup belongs to a catalog/scope; diagnostics use registry snapshots. + +The `TexturePtr` control block uses a factory-controlled deleter. Final release appends the raw +texture to TextureFactory's released queue; it does not execute `Texture::~Texture()` immediately. +`Window::display()` flushes pending batches and then calls +`TextureFactory::collectReleasedTextures()` while the active context is current. Engine shutdown +performs the same collection explicitly because no later display is guaranteed. + +## 5. GPU lifetime and threading + +### 5.1 Graphics-thread lifetime contract + +GPU resources are graphics-thread-affine. Creating, mutating and finally releasing owning handles +must follow the engine's graphics/update lifecycle. `std::shared_ptr` provides ownership safety; it +does not expand eepp's supported threading contract. Async CPU decoding may run elsewhere, but +ownership handoff and final release are marshalled to the main/scene update path unless an existing +API explicitly acquires a shared GL context. + +Debug builds should assert this contract at factory release and collection boundaries. The design +does not add a generic device state, epoch or arbitrary-thread disposal queue for unsupported usage. + +### 5.2 Texture deferred destruction + +Final `TexturePtr` release queues the Texture object in TextureFactory. It remains allocated until a +safe collection point: + +```cpp +void Window::display( bool clear ) { + GlobalBatchRenderer::instance()->draw(); + TextureFactory::instance()->collectReleasedTextures(); + swapBuffers(); + // ... +} +``` + +Batch flushing precedes collection because the current renderer still keeps borrowed texture state. +Long-lived render queues must eventually retain TexturePtr themselves. The released queue is also +drained during Engine shutdown after consumers are released and before TextureFactory, Renderer or +contexts are destroyed. + +Other self-contained GPU classes retain their direct, graphics-thread destruction model. They are +not routed through TextureFactory and do not acquire generic lifetime machinery unless a later +ownership migration demonstrates a concrete need. + +### 5.3 Resource mutation and loading threads + +- Decode and network work may run off-thread. +- GPU create/upload/reload/mutation and final ownership release obey the graphics-thread/shared- + context contract. +- Registries and caches are thread-safe at their boundaries. +- UI mutation executes on the scene/main thread and remains protected by scene generation tokens. +- A generation token controls whether a subscriber may mutate a scene; it does not own a resource or + a shared request. +- No callback, resource destruction, or device command executes while a registry/cache mutex is held. + +## 6. Engine shutdown and restart contract + +Engine teardown must be reordered around producer shutdown, consumer release and valid GL contexts: + +1. Mark Engine and Web cache/resource delivery services as shutting down; reject new work. +2. Invalidate scene/document async subscribers and stop accepting main-thread resource deliveries. +3. Cancel/stop and join resource/network/decode producers that can create resources or callbacks. +4. Destroy scenes, documents, UI resolvers, document leases, scene scopes, and application caches. +5. Clear application/global catalogs and remaining manager-owned handles. +6. Flush/discard pending rendering submissions, then collect TextureFactory's released textures. +7. Inspect the weak texture registry. Debug/tests assert that no unexpected strong TexturePtr + remains; a defensive shutdown sweep may release the GPU payload of reported survivors. +8. Destroy device-dependent managers in audited order while Renderer and contexts remain valid. +9. Destroy TextureFactory, Renderer, windows/contexts and backend state in that order. + +The exact manager list will be produced by the Stage 1 GPU audit, but these order constraints are +fixed: + +- HTTP/decode producers stop before resource consumers and GPU systems are dismantled. +- `ShaderProgramManager` releases programs before Renderer/GL dispatch is destroyed. +- TextureFactory's deferred released queue is empty before its context disappears. +- A TexturePtr surviving Engine destruction is a project-contract violation, reported by debug + builds and tests rather than supported through a second device-lifetime architecture. +- Destructors cannot recreate singletons. +- Tests release all resource handles before recreating Engine state from zero. + +## 7. Drawable model + +Shared lifetime and shareable instance state are separate concerns. `isStateful()` is not a sharing +contract and will not be used as one. + +### 7.1 Frozen source/instance split + +Resource resolution caches immutable source data. UI consumers own per-consumer drawable instances: + +```cpp +using DrawableSourcePtr = ResourcePtr; +using DrawablePtr = ResourcePtr; + +DrawableSourcePtr DrawableResolver::findSource( const DrawableRequest& request ); +DrawablePtr DrawableResolver::createDrawable( const DrawableRequest& request ); +``` + +Representative split: + +- `Texture` is shared GPU/resource data, not a globally shared mutable drawable instance. +- `TextureRegionSource` contains a TexturePtr, immutable source rectangle, offset, and intrinsic size. +- `NinePatchSource` contains immutable region and border data. +- `TextureDrawable`/`TextureRegionDrawable` hold per-consumer destination size, tint, alpha, position, + and other presentation state while retaining their source. +- `StateListDrawable`, `DrawableGroup`, and `Sprite` are per-consumer state machines/instances that + refer to source handles or private child instances. + +`DrawableImageParser::createDrawable()` always returns a fresh consumer instance for CSS-generated +or resolved content, even when its immutable source came from a cache. + +The migration will remove draw-time mutation of shared child/source objects. Rendering APIs may use +external draw parameters where that simplifies an implementation, but no shared source can be +temporarily recolored, resized, repositioned, or advanced by a consumer. + +### 7.2 Consumer API + +Consumers store a strong per-consumer instance: + +```cpp +UIImage* UIImage::setDrawable( DrawablePtr drawable ); +DrawablePtr UIImage::getDrawable() const; +``` + +The same rule applies to CSS layers, buttons, skins, icon instances, state lists, and groups. All +`ownIt` flags and manual drawable deletion paths are removed in the same drawable migration stage. + +`Models::Variant` will be structurally migrated to `std::variant` (or an equivalently safe +non-union representation) with `DrawablePtr` as a normal non-trivial member. Its old owning flag and +raw drawable alternative are removed. + +### 7.3 Resource notifications + +`DrawableResource::Unload` is removed as a consumer lifetime mechanism. A strong owner cannot be +notified that its object vanished, and destructor callbacks into a partially destroyed most-derived +object are unsafe. + +Mutable/reloadable source data uses a typed change/invalidation signal with RAII connection tokens. +Callbacks capture weak lifetime tokens rather than raw consumer `this` pointers. Registry expiration +is observed through weak handles and snapshots, not an object destructor callback. + +## 8. UI resolution layering + +`DrawableSearcher` is replaced, but not by putting all of its behavior into Graphics::ResourceScope. + +`UI::DrawableResolver` owns UI-specific interpretation: + +- CSS gradients and functions +- `url(...)` and scene-relative URI resolution +- icons, glyphs, sprites, and generated drawable instances +- node/scene context and UI source-to-instance creation + +It delegates texture/source lookup and creation to the scene's `Graphics::ResourceScope`. A default +UI resolver can use the default Graphics scope for UI applications without custom scenes, but pure +Graphics does not depend on it. + +Browser request policy, cookies, navigation, and cache leases are not responsibilities of either +DrawableResolver or Graphics::ResourceScope. + +## 9. Web document ownership and shared cache + +### 9.1 Topology + +Each UIWebView document has a distinct `DocumentSessionId`, document scope, and cache lease. Multiple +documents may use one shared WebResourceCache: + +```text +Document scope/session A ─┐ + ├── shared WebResourceCache partition +Document scope/session B ─┘ +``` + +Navigation changes/releases only that document's lease. It never directly purges entries required +by another document. Widgets retain their currently displayed resource/source through ordinary +strong handles independently of cache retention. + +### 9.2 Cache key and isolation + +```cpp +struct OriginKey { + std::string scheme; + std::string normalizedHost; + Uint16 effectivePort; +}; + +struct WebResourceKey { + CachePartitionId partition; + CanonicalURI uri; + ResourceKind kind; + DecodeOptions decode; + RequestVariant requestVariant; +}; +``` + +`CachePartitionId` represents the intentionally shared HTTP/cookie/authentication context. Different +cookie jars or credential contexts do not share entries merely because a URI matches. Request +variants account for content-affecting headers until full HTTP `Vary` support exists. + +Canonicalization rules are fixed at the cache boundary: + +- Same-origin means normalized scheme, host, and effective port all match. +- URI fragments are removed; query strings remain part of the key. +- Redirect metadata records both request URI and canonical final URI without merging partitions. +- File paths use platform-aware canonicalization with explicit symlink/case behavior. +- Data URIs use a content hash plus decode options and enforce resource-size limits. +- Hashes accelerate lookup but full keys determine equality. + +### 9.3 Entry state and request coalescing + +```cpp +enum class LoadState { Empty, Loading, Ready, Failed, Cancelled }; + +struct WebCacheEntry { + WebResourceKey key; + LoadState state; + TexturePtr retainedResource; + TextureWeakPtr liveResource; + MonotonicTime lastUsed; + MonotonicTime expiresAt; + std::size_t retainedBytes; + UnorderedSet activeLeases; + std::vector subscribers; +}; +``` + +Concurrent requests for one key share one fetch/decode/upload operation. Each subscriber has its own +document session and scene generation. A stale subscriber is removed without cancelling delivery to +other current subscribers. Cancellation of the shared operation occurs only when policy permits and +no subscriber/cache requirement remains. + +Retention uses monotonic TTL, LRU information, and per-partition/global byte budgets. Same-origin +navigation may renew a document lease; cross-origin navigation releases that document's old-origin +lease. External consumer handles remain valid regardless of cache eviction. + +Failure entries define retry/backoff and do not become permanent accidental cache hits. + +## 10. Migration strategy + +No externally released intermediate state is required. Temporary duplicated retention is allowed on +the feature branch to keep behavior valid while the repository-wide API break is assembled. + +### Stage 0: contract freeze and inventories + +Status: complete. See `resource_shared_ownership_stage0_inventory.md`. + +Deliverables: + +- This architecture document accepted or amended. +- Complete inventory of GPU resource classes and direct GL deletion sites. +- Complete inventory of stored raw `Texture*`, texture-ID lookup, ignored texture-load returns, + loader callback signatures, and factory deletion calls. +- Complete drawable mutation/shareability inventory. +- Build-matrix decision for shared/static libraries and `EE_MEMORY_MANAGER`. +- Concrete shutdown dependency graph for Engine-owned producers, consumers, managers, Renderer, and + contexts. + +Exit criterion: no unresolved ownership, lookup, last-release-thread, Engine restart, scope import, +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: + +- 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. +- TextureAtlasLoader joins/stops ResourceLoader work before callback-visible loader state is + destroyed. +- 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. + +### Stage 1: texture lifetime scaffolding, with old factory retention still active + +Implement stable ResourceId, ResourceMetrics, centralized eepp-compatible handle creation, the weak +TextureFactory live registry, TextureFactory's deferred released-texture queue, shutdown diagnostics, +and graphics-thread assertions. Integrate collection into Window::display() after batch flush and +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. + +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. + +Exit tests: + +- Released textures are deleted only at display/final shutdown collection points. +- Pending batches flush before texture collection. +- Engine teardown leaves no pending released textures or unexpected live registry entries. +- Repeated test-only Engine create/destroy cycles start with empty resource state. +- Wrong-thread final release triggers the documented debug contract assertion. +- `EE_MEMORY_MANAGER` accurately removes texture allocations through the factory-controlled deleter. + +### Stage 2: one complete TexturePtr ownership cut + +Change creation/acquisition APIs to return TexturePtr and migrate every required holder in the same +repository-wide cut. During conversion, TextureFactory temporarily retains strong handles so an +unclassified ignored result cannot silently expire. + +At minimum migrate: + +- TextureLoader state, return types, callbacks, unload behavior, and async captures +- Texture GIF frame ownership +- TextureRegion and region-source texture ownership +- TextureAtlas, TextureAtlasLoader, and texture vectors +- FrameBuffer attachment ownership +- fonts and glyph texture caches +- nine-patches and region chains +- 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 +- eepp, ecode, modules, examples, tools, and tests + +Every stored raw Texture pointer is classified as strong, weak, or a short borrow dominated by a +documented owner. Add a source audit/clang-tidy check where practical. + +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. + +### Stage 3: catalog and scope ownership cutover + +Implement the global catalog, default Graphics scope, application/scene catalogs, explicit imports, +immutable keys, and aliases. Move intended persistent resources from temporary factory retention into +catalogs/caches. Remove factory-wide strong retention and activate final unpinned creation. + +Remove semantic TextureFactory name/hash lookup and migrate every lookup to a scope/catalog. + +Exit criteria: + +- Dropping the last real consumer/catalog/cache handle expires a texture. +- Duplicate names in unrelated scopes resolve independently. +- Sibling/document resources are invisible without explicit catalog import. +- Global resources persist only because the global catalog owns them. +- Live diagnostic records cannot be resolved semantically. +- Scope destruction does not invalidate externally retained resources. + +### Stage 4: drawable source/instance conversion + +Introduce source types and per-consumer instances, remove shared draw-state mutation, replace manual +ownership with DrawablePtr, remove Unload lifetime callbacks, add RAII change connections, and +migrate Variant's storage. + +Convert UIImage, UINodeDrawable layers, StateListDrawable, DrawableGroup, Sprite, UIPushButton, +UISkin, UIIcon, themes, and DrawableImageParser in one coherent cut. + +Exit criteria: + +- Two consumers using one source have independent tint, alpha, size, position, state, and animation. +- Nested/reentrant drawing leaves no shared source or child modified. +- Drawable groups do not reposition shared child instances. +- Variant copying cannot duplicate exclusive ownership. +- No drawable `ownIt` or manual child deletion remains. + +### Stage 5: layered UI resolution + +Implement UI::DrawableResolver and replace DrawableSearcher. Scene resolvers delegate Graphics work +to their explicit scope. Keep CSS/icon/glyph interpretation in UI and cookie/navigation concerns in +Web services. + +Exit criteria: + +- Pure Graphics works without UI. +- UI resolves through its scene/application scope. +- Embedded documents cannot see sibling resources accidentally. +- Host/application assets require explicit export/import. + +### Stage 6: WebResourceCache and document leases + +Implement cache partitions, canonical keys/origins, per-document sessions and leases, in-flight +coalescing, per-subscriber generation guards, retries, TTL/LRU, and byte budgets. Integrate WebView +navigation at its existing document replacement boundary. + +Exit criteria: + +- Shared tabs reuse eligible resources without sharing document ownership. +- Navigation in one tab cannot evict another tab's active lease. +- Different cookie/auth partitions cannot reuse credential-dependent responses. +- Stale subscribers do not block current subscribers. +- Same-origin retention and cross-origin lease release follow policy. +- Cache memory remains within configured budgets. + +### Stage 7: remaining resource families + +Migrate fonts, font faces/fallback caches, themes, shader programs/shaders, nine-patch catalogs, +atlas managers, and every remaining raw-owning ResourceManager subclass one family at a time. Their +self-contained GPU objects retain the established graphics-thread destruction contract unless a +concrete migration requires otherwise. + +Remove raw-owning `ResourceManager` only when no subclass or consumer depends on it. + +## 11. Required validation matrix + +### Ownership and registry + +- Unpinned texture expires after its last real owner releases it. +- Region/source retains its texture; atlas retains its sources/textures; framebuffer retains its + attachment. +- 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 + +- Final texture release on the graphics thread queues rather than immediately deleting. +- Wrong-thread final release is detected as a project-contract violation in debug builds. +- 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. + +### Scope isolation + +- Identical keys in sibling scopes resolve independently. +- Explicit imported catalog resolves; missing import does not fall back. +- Global catalog is visible only to scopes importing it. +- Registry records are never semantically resolvable. +- Scope/catalog destruction leaves externally retained resources alive. +- Catalog import order is deterministic and graph cycles are structurally impossible. + +### Drawable independence + +- One source displayed by two widgets with different tint, alpha, and size. +- Two region instances draw at different sizes without shared mutation. +- State-list and sprite instances maintain independent state/time. +- Nested/reentrant drawing restores nothing because shared objects were not mutated. +- DrawableGroup owns/private-instantiates mutable children. +- Variant copy/move/reset is safe with DrawablePtr. + +### Web cache + +- Same-origin navigation reuses resources. +- Cross-origin navigation releases only the navigating document lease. +- Two tabs share eligible entries and survive independent navigation. +- Different cookie/auth partitions do not share. +- URI plus different decode/request options produces distinct entries. +- Concurrent requests coalesce while subscribers retain independent generation guards. +- Failed requests retry according to explicit policy. +- TTL uses a monotonic clock; byte-budget eviction works independently. + +### Teardown and repeatable tests + +- Pending HTTP, decode, and upload work during scene and Engine destruction. +- No destructor recreates Engine, Renderer, TextureFactory, or another manager. +- Repeated Engine create/destroy cycles in one process. +- Each unit test releases its scopes, catalogs, leases, and resources independently. +- Test teardown reports unintended catalog pins and live resource provenance. +- `EE_MEMORY_MANAGER`, supported debug/release configurations, static builds, and shared-library + builds. + +## 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. + +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. diff --git a/.agent/plans/resource_shared_ownership_stage0_inventory.md b/.agent/plans/resource_shared_ownership_stage0_inventory.md new file mode 100644 index 000000000..ef3381492 --- /dev/null +++ b/.agent/plans/resource_shared_ownership_stage0_inventory.md @@ -0,0 +1,526 @@ +# 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. +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` and `VertexBufferManager : Container` + 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 +``` + +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. diff --git a/src/eepp/network/http.cpp b/src/eepp/network/http.cpp index d18187b78..570dfbc74 100644 --- a/src/eepp/network/http.cpp +++ b/src/eepp/network/http.cpp @@ -1685,8 +1685,14 @@ Http::Pool::~Pool() { } void Http::Pool::clear() { - Lock l( mMutex ); - mHttps.clear(); + decltype( mHttps ) https; + { + Lock l( mMutex ); + https.swap( mHttps ); + } + // Http destruction joins local request threads and callbacks can re-enter the global pool. + // Never run either operation while holding the pool mutex. + https.clear(); } std::string Http::Pool::getHostKey( const URI& host, const URI& proxy ) { diff --git a/src/tests/unit_tests/http.cpp b/src/tests/unit_tests/http.cpp index caf3e9404..79c780439 100644 --- a/src/tests/unit_tests/http.cpp +++ b/src/tests/unit_tests/http.cpp @@ -1,6 +1,9 @@ #include "utest.h" #include +#include +#include +#include #include #include @@ -78,6 +81,93 @@ UTEST( Http, responseHeaderLineLargerThanReceiveBuffer ) { EXPECT_TRUE( response.getBody() == "hello" ); } +UTEST( Http, poolClearAllowsCallbackReentry ) { + Http::setThreadPool( nullptr ); + Http::Pool::getGlobal().clear(); + + TcpListener listener; + ASSERT_EQ( listener.listen( Socket::AnyPort, IpAddress::LocalHost ), Socket::Done ); + + std::thread server( [&listener] { + TcpSocket client; + if ( listener.accept( client ) != Socket::Done ) + return; + + std::string request; + char buffer[1024]; + std::size_t received = 0; + while ( request.find( "\r\n\r\n" ) == std::string::npos ) { + if ( client.receive( buffer, sizeof( buffer ), received ) != Socket::Done ) + return; + request.append( buffer, received ); + } + + const std::string response = "HTTP/1.1 200 OK\r\n" + "Content-Length: 2\r\n" + "Connection: close\r\n\r\n" + "ok"; + client.send( response.data(), response.size() ); + client.disconnect(); + } ); + + const URI uri( String::format( "http://127.0.0.1:%u/", listener.getLocalPort() ) ); + auto http = Http::Pool::getGlobal().get( uri ); + std::mutex callbackMutex; + std::condition_variable callbackCondition; + bool callbackEntered = false; + bool allowPoolReentry = false; + bool callbackCompleted = false; + bool callbackResponseOk = false; + + http->sendAsyncRequest( + [&]( const Http&, Http::Request&, Http::Response& response ) { + { + std::unique_lock lock( callbackMutex ); + callbackResponseOk = response.getStatus() == Http::Response::Ok; + callbackEntered = true; + callbackCondition.notify_all(); + callbackCondition.wait( lock, [&] { return allowPoolReentry; } ); + } + + // Pool::clear() is concurrently destroying and joining this Http. It must not hold the + // Pool mutex while waiting for this callback. + Http::Pool::getGlobal().get( uri ); + { + std::lock_guard lock( callbackMutex ); + callbackCompleted = true; + } + callbackCondition.notify_all(); + }, + Http::Request( "/" ), Seconds( 5 ) ); + + http.reset(); // Leave the Pool as the only Http owner. + { + std::unique_lock lock( callbackMutex ); + ASSERT_TRUE( callbackCondition.wait_for( lock, std::chrono::seconds( 5 ), + [&] { return callbackEntered; } ) ); + } + + std::thread clearThread( [] { Http::Pool::getGlobal().clear(); } ); + std::this_thread::sleep_for( std::chrono::milliseconds( 50 ) ); + { + std::lock_guard lock( callbackMutex ); + allowPoolReentry = true; + } + callbackCondition.notify_all(); + + { + std::unique_lock lock( callbackMutex ); + ASSERT_TRUE( callbackCondition.wait_for( lock, std::chrono::seconds( 5 ), + [&] { return callbackCompleted; } ) ); + } + + clearThread.join(); + server.join(); + listener.close(); + Http::Pool::getGlobal().clear(); + EXPECT_TRUE( callbackResponseOk ); +} + #if EE_PLATFORM != EE_PLATFORM_WIN UTEST( Http, tcpConnectTimeoutHandlesFdAboveFdSetSize ) { TcpListener listener;