Repository navigation
Conversation
… tiles An MBTiles file's metadata (its credits, and what the layer needs to draw) is read on a thread of its own, which didn't count as under way. An export's complete render stops waiting once nothing is in flight and nothing more lands, so it could find the metadata still loading, nothing in flight, and give up within milliseconds with the layer missing. The read now counts as under way from before its thread starts until what it read is kept, as a PMTiles header read already does. Fixes opengeos#1801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. Walkthrough
Merge Risk:
 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Over once what it read is kept, before its waiters wake: whoever sees it over | ||
| // sees the metadata, and a caller that waited sees it no longer counted. | ||
| drop(under_way); | ||
| pending.lock().unwrap_or_else(|e| e.into_inner()).remove(&worker_key); | ||
| ready.notify_all(); |
There was a problem hiding this comment.
Possible lost wake-up (confidence: moderate, ~40%). drop(under_way) runs finished(), which bumps the generation, decrements IN_FLIGHT and wakes the map, all before pending.remove and ready.notify_all(). The old trailing crate::cache::notify() (which came after the removal) is also gone, so the last wake now comes before the key leaves the pending set.
When remember_info doesn't store the result (the epoch moved, or check_still refused, as the test at mbtiles.rs:475 arranges), a render woken by that bump can run in the gap and see the key still pending. info_ready then returns Ok(None) and nothing wakes the map after the removal. The export loop recovers because its next render re-spawns the read. The interactive map has no such recovery in this path.
Dropping the count last fixes the ordering: anyone who sees in_flight() == 0 also sees the read off the pending set, and the final wake follows the removal.
| // Over once what it read is kept, before its waiters wake: whoever sees it over | |
| // sees the metadata, and a caller that waited sees it no longer counted. | |
| drop(under_way); | |
| pending.lock().unwrap_or_else(|e| e.into_inner()).remove(&worker_key); | |
| ready.notify_all(); | |
| // Over last: once the read is off the pending set and its waiters are woken, so the | |
| // wake that ends the count follows the removal, and whoever sees it over sees both. | |
| pending.lock().unwrap_or_else(|e| e.into_inner()).remove(&worker_key); | |
| ready.notify_all(); | |
| drop(under_way); |
Code reviewThe change is small and sound in its main idea: an MBTiles metadata read now counts as under way, so an export waits for it instead of giving up. I found one ordering concern, posted inline. Prior reviews: none, so no earlier concerns were left alone. Bugs
Security
Performance
Quality
AGENTS.md
Change sizeCompared with main (merge base b373948 to PR head 5f54546).
|
Summary
An MBTiles file's metadata (its credits, and what the layer needs before it can draw) is read on a thread of its own, and that read didn't count as under way in
cache::in_flight(). An export's complete render (render::complete) stops waiting once nothing is in flight and nothing more lands, so it could find the metadata still loading, nothing in flight, and give up within milliseconds: the layer came out as a gap ("some tiles were still on their way when the export stopped waiting…").The read now counts as under way from before its thread starts until what it read is kept, as a PMTiles header read (
fetch_local) already does.cache::UnderWayis that count: it ends, and wakes the map, when dropped, so a thread that fails to start or a read that panics can't leave it stuck. It is native only; the browser reads the metadata in place.Testing
mbtiles::tests::a_metadata_read_counts_as_under_way_while_it_runslooks atin_flight()from inside the metadata worker (through the existingStillcheck), so it doesn't depend on timing. It fails without the fix ("the worker's own read wasn't counted as under way") and passes with it.metadata_ready_checks_still_outside_the_pending_locknow waits for the read it starts, so the count is back before the next test that holds the cache's turn comparesin_flight().cargo test -p rustgis-tiles: 436 passed. fmt, clippy and the wasm build check ofrustgis-tiles.rustgis-enginelib run in 8 before), 12 full runs with this fix all passed: https://github.com/Mavrick-F/RustGIS/actions/runs/43084074971 (the Windows x64 job's log was lost by GitHub; it is being run again).UI evidence
No visible UI change.
Related issue
Fixes #1801. Part of getting the nightly green again (#1776).
🤖 Generated with Claude Code
Summary by CodeRabbit