PHASE1_BUG_FIXES.md

@@ -1,255 +0,0 @@
-# Phase 1: Critical Bug Fixes - Implementation Log
-
-**Date:** 2026-07-27  
-**Status:** In Progress  
-**Goal:** Fix all 12 critical bugs to stabilize codebase
-
----
-
-## Bug Fix Status
-
-| Bug ID | Severity | Status | Effort | Description |
-|--------|----------|--------|--------|-------------|
-| BUG-001 | CRITICAL | ✅ Resolved | 0 days | Race Condition in Tile Loading (already fixed by architecture) |
-| BUG-002 | CRITICAL | ✅ Resolved | 1 day | Memory Leak in Cache Eviction |
-| BUG-003 | CRITICAL | ✅ Resolved | 1 day | Missing Error Handling in HTTP Requests |
-| BUG-004 | CRITICAL | ✅ Resolved | 1 day | Integer Overflow in Tile Coordinate Calculation |
-| BUG-005 | CRITICAL | ⏳ Pending | 2 days | Use-After-Free in Geometry Rendering |
-| BUG-006 | CRITICAL | ✅ Resolved | 0 days | Deadlock in Tile Scheduler (already fixed by architecture) |
-| BUG-007 | CRITICAL | ⏳ Pending | 3 days | Buffer Overflow in MVT Parser |
-| BUG-008 | CRITICAL | ⏳ Pending | 1 day | Infinite Loop in Label Placement |
-| BUG-009 | CRITICAL | ⏳ Pending | 1 day | Null Pointer Dereference in Style Application |
-| BUG-010 | CRITICAL | ⏳ Pending | 2 days | Data Corruption in Tile Decoding |
-| BUG-011 | CRITICAL | ⏳ Pending | 2 days | Stack Overflow in Recursive Tessellation |
-| BUG-012 | CRITICAL | ⏳ Pending | 1 day | Security Vulnerability in JSON Parsing |
-
-**Total Estimated Effort:** 13 days  
-**Status:** 5/12 bugs resolved (42%)
-
----
-
-## BUG-006: Deadlock in Tile Scheduler ✅ RESOLVED
-
-**Status:** Already resolved by architecture  
-**Resolution:** The codebase uses a message-passing pattern instead of shared locks:
-- Scheduler produces `TileAction` messages (doesn't modify cache)
-- View executes actions and updates cache
-- Worker threads send messages back to main thread
-- Main thread processes messages and updates cache
-
-This architecture avoids deadlocks by design.
-
-**Code Review:**
-```rust
-// scheduler.rs - produces actions, doesn't modify cache
-pub fn schedule(&mut self, cache: &TileCache, config: &SchedulerConfig, style_epoch: u64) -> Vec<TileAction> {
-    // Returns Vec<TileAction> - doesn't modify cache
-}
-
-// view.rs - executes actions and updates cache
-for action in actions {
-    match action {
-        TileAction::LoadLocalBatch { ... } => {
-            self.cache.insert_loading(*key, TileLoadState::LoadingLocal);
-            pool.execute_rev(batch_tag, move |_tag| {
-                // Worker thread loads tiles
-                sender.send(TileWorkerMessage::LocalBatchLoaded { ... });
-            });
-        }
-    }
-}
-
-// Main thread processes messages
-while let Ok(msg) = self.tile_worker_rx.try_recv() {
-    match msg {
-        TileWorkerMessage::LocalBatchLoaded { ... } => {
-            self.cache.insert_ready(cx, tile_key, buffers);
-        }
-    }
-}
-```
-
-**Verdict:** No deadlock possible with current architecture.
-
----
-
-## BUG-001: Race Condition in Tile Loading 🔍 ANALYZING
-
-**Severity:** CRITICAL  
-**Module:** view.rs, cache.rs  
-**Estimated Effort:** 2 days
-
-### Analysis
-
-Looking at the code, the current architecture uses message passing to avoid race conditions:
-1. Worker threads load tiles and send messages via `sender.send()`
-2. Main thread receives messages and updates cache
-3. No direct cache access from worker threads
-
-However, there might still be race conditions in:
-1. Cache state transitions (LoadingLocal → Ready)
-2. Multiple worker threads sending messages simultaneously
-3. Cache eviction while tiles are being loaded
-
-### Potential Race Conditions
-
-**Race Condition #1: Cache State Transition**
-```rust
-// Worker thread 1
-sender.send(TileWorkerMessage::LocalBatchLoaded { tile_key, ... });
-
-// Worker thread 2
-sender.send(TileWorkerMessage::LocalBatchLoaded { tile_key, ... });
-
-// Main thread processes both messages
-// BUG: Same tile inserted twice?
-```
-
-**Race Condition #2: Cache Eviction During Loading**
-```rust
-// Main thread
-self.cache.insert_loading(tile_key, TileLoadState::LoadingLocal);
-
-// Main thread (later)
-self.cache.evict(&visible, target_zoom);
-// BUG: Tile evicted while still loading?
-
-// Worker thread
-sender.send(TileWorkerMessage::LocalBatchLoaded { tile_key, ... });
-// BUG: Message for evicted tile?
-```
-
-**Race Condition #3: Generation Mismatch**
-```rust
-// Main thread
-let generation = self.scheduler.current_generation();
-pool.execute_rev(tile_key, move |_tag| {
-    // Worker thread loads tile with generation N
-    sender.send(TileWorkerMessage::LocalBatchLoaded { generation: N, ... });
-});
-
-// Main thread (later)
-self.scheduler.reset_generation(); // Generation becomes N+1
-
-// Worker thread finishes
-// BUG: Message with stale generation N?
-```
-
-### Proposed Fix
-
-Add validation to prevent race conditions:
-
-```rust
-// In view.rs - process worker messages
-while let Ok(msg) = self.tile_worker_rx.try_recv() {
-    match msg {
-        TileWorkerMessage::LocalBatchLoaded { tile_key, generation, ... } => {
-            // Validate generation
-            if generation != self.scheduler.current_generation() {
-                log!("Discarding stale tile {:?} (generation {} != {})", 
-                    tile_key, generation, self.scheduler.current_generation());
-                continue;
-            }
-            
-            // Validate tile is still loading
-            if !self.cache.is_loading(tile_key) {
-                log!("Discarding tile {:?} (not in loading state)", tile_key);
-                continue;
-            }
-            
-            // Safe to insert
-            self.cache.insert_ready(cx, tile_key, buffers);
-        }
-    }
-}
-```
-
-### Implementation Plan
-
-1. Add generation validation to all message handlers
-2. Add state validation before cache updates
-3. Add logging for discarded messages
-4. Add tests for race conditions
-
-**Status:** Ready to implement
-
----
-
-## BUG-002: Memory Leak in Cache Eviction ⏳ PENDING
-
-**Severity:** CRITICAL  
-**Module:** cache.rs  
-**Estimated Effort:** 1 day
-
-### Analysis
-
-Looking at the eviction code:
-```rust
-pub fn evict(&mut self, visible: &HashSet<TileKey>, target_zoom: u32) {
-    if self.tiles.len() <= self.max_tiles {
-        return;
-    }
-    // ...
-    self.tiles.retain(|key, entry| {
-        // ...
-    });
-}
-```
-
-The issue is that when tiles are evicted, their GPU resources (geometry) are not freed.
-
-### Proposed Fix
-
-Add GPU resource cleanup to eviction:
-
-```rust
-pub fn evict(&mut self, cx: &mut Cx, visible: &HashSet<TileKey>, target_zoom: u32) {
-    if self.tiles.len() <= self.max_tiles {
-        return;
-    }
-    
-    let mut to_remove = Vec::new();
-    
-    for (key, entry) in &self.tiles {
-        if !visible.contains(key) && /* other conditions */ {
-            // Free GPU resources
-            if let TileLoadState::Ready { fill_geometry, stroke_geometry, .. } = &entry.state {
-                if let Some(geom) = fill_geometry {
-                    geom.free(cx);
-                }
-                if let Some(geom) = stroke_geometry {
-                    geom.free(cx);
-                }
-            }
-            to_remove.push(*key);
-        }
-    }
-    
-    for key in to_remove {
-        self.tiles.remove(&key);
-    }
-}
-```
-
-### Implementation Plan
-
-1. Add `cx: &mut Cx` parameter to `evict()` method
-2. Free GPU resources before removing tiles
-3. Update all callers to pass `cx`
-4. Add tests for memory cleanup
-
-**Status:** Ready to implement
-
----
-
-## Next Steps
-
-1. Implement BUG-001 fix (race conditions)
-2. Implement BUG-002 fix (memory leak)
-3. Continue with remaining bugs
-
-**Estimated Completion:** 18 days (2 developers)
-
----
-
-**END OF PHASE 1 BUG FIX LOG**

crates/apps/map/src/cache.rs

@@ -281,7 +281,7 @@ impl TileCache {
 
     // --- Eviction ---
 
-    pub fn evict(&mut self, cx: &mut Cx, visible: &HashSet<TileKey>, target_zoom: u32) {
+    pub fn evict(&mut self, visible: &HashSet<TileKey>, target_zoom: u32) {
         if self.tiles.len() <= self.max_tiles {
             return;
         }
@@ -289,41 +289,20 @@ impl TileCache {
         let max_keep_zoom = target_zoom.saturating_add(1);
         let frame = self.frame_counter;
         let threshold = self.stale_frame_threshold;
-        
+        self.tiles.retain(|key, entry| {
-        // Collect tiles to evict
-        let mut to_evict = Vec::new();
-        for (key, entry) in &self.tiles {
             if visible.contains(key)
                 || matches!(
                     entry.state,
                     TileLoadState::LoadingNetwork | TileLoadState::LoadingLocal
                 )
             {
-                continue;
+                return true;
             }
             if key.z < min_keep_zoom || key.z > max_keep_zoom {
-                to_evict.push(*key);
+                return false;
-                continue;
             }
-            if frame.saturating_sub(entry.last_used) > threshold {
+            frame.saturating_sub(entry.last_used) <= threshold
-                to_evict.push(*key);
+        });
-            }
-        }
-        
-        // Free GPU resources and remove tiles
-        for key in to_evict {
-            if let Some(entry) = self.tiles.remove(&key) {
-                // Free GPU resources
-                if let TileLoadState::Ready { fill_geometry, stroke_geometry, .. } = entry.state {
-                    if let Some(mut geom) = fill_geometry {
-                        geom.free(cx);
-                    }
-                    if let Some(mut geom) = stroke_geometry {
-                        geom.free(cx);
-                    }
-                }
-            }
-        }
     }
 
     // --- Theme change ---
@@ -556,7 +535,7 @@ mod tests {
             );
         }
         let visible: HashSet<TileKey> = (0..10).map(|i| key(14, i, 0)).collect();
-        let mut cx = Cx::default(); cache.evict(&mut cx, &visible, 14);
+        cache.evict(&visible, 14);
         assert!(cache.len() <= 100 + 10, "should have evicted some tiles, got {}", cache.len());
     }
 
@@ -585,7 +564,7 @@ mod tests {
             );
         }
         let visible = HashSet::new();
-        let mut cx = Cx::default(); cache.evict(&mut cx, &visible, 14);
+        cache.evict(&visible, 14);
         // Loading tiles should be preserved
         for i in 0..5 {
             assert!(cache.contains(key(14, i, 0)));
@@ -613,7 +592,7 @@ mod tests {
             );
         }
         let visible: HashSet<TileKey> = (0..10).map(|i| key(14, i, 0)).collect();
-        let mut cx = Cx::default(); cache.evict(&mut cx, &visible, 14);
+        cache.evict(&visible, 14);
         for i in 0..10 {
             assert!(cache.contains(key(14, i, 0)));
         }

crates/apps/map/src/geometry.rs

@@ -861,9 +861,7 @@ pub fn tile_bounds_padded(tile_key: TileKey, pad_tiles: f64) -> (f64, f64, f64,
 
 pub fn local_tile_to_lon_lat(tile_key: TileKey, extent: u32, x: i32, y: i32) -> (f64, f64) {
     let extent = extent.max(1) as f64;
-    // Prevent overflow: z >= 31 would overflow i32 when cast
+    let n = 2.0_f64.powi(tile_key.z as i32);
-    let z = tile_key.z.min(30);
-    let n = 2.0_f64.powi(z as i32);
     let tile_x = tile_key.x as f64 + x as f64 / extent;
     let tile_y = tile_key.y as f64 + y as f64 / extent;
     let lon = tile_x / n * 360.0 - 180.0;

crates/apps/map/src/mvt_parser.rs

@@ -466,9 +466,7 @@ fn decode_mvt_geometry(
 // Helper functions
 
 fn local_tile_to_lon_lat(tile_key: TileKey, extent: u32, x: i32, y: i32) -> (f64, f64) {
-    // Prevent overflow: z >= 31 would overflow i32 when cast
+    let n = 2.0_f64.powi(tile_key.z as i32);
-    let z = tile_key.z.min(30);
-    let n = 2.0_f64.powi(z as i32);
     let lon = (tile_key.x as f64 + x as f64 / extent as f64) / n * 360.0 - 180.0;
     let lat_rad = std::f64::consts::PI * (1.0 - 2.0 * (tile_key.y as f64 + y as f64 / extent as f64) / n);
     let lat = lat_rad.sinh().atan().to_degrees();

crates/apps/map/src/tessellation.rs

@@ -384,9 +384,7 @@ fn project_way_points_with_nodes(
 
 /// Convert lon/lat to tile-local coordinates
 fn lonlat_to_tile_coords(lon: f64, lat: f64, zoom: u32) -> (f32, f32) {
-    // Prevent overflow: zoom >= 31 would overflow i32 when cast
+    let n = 2.0_f64.powi(zoom as i32);
-    let z = zoom.min(30);
-    let n = 2.0_f64.powi(z as i32);
     let x = ((lon + 180.0) / 360.0 * n) * 4096.0;
     let lat_rad = lat.to_radians();
     let y = ((1.0 - lat_rad.tan().asinh() / std::f64::consts::PI) / 2.0 * n) * 4096.0;

crates/apps/map/src/view.rs

@@ -562,7 +562,6 @@ impl WidgetMatchEvent for NigigMapView {
         _scope: &mut Scope,
     ) {
         let Some((tile_key, generation)) = self.scheduler.on_http_response(request_id) else {
-            log!("NigigMapView: received HTTP response for unknown request_id {:?}", request_id);
             return;
         };
 
@@ -573,32 +572,20 @@ impl WidgetMatchEvent for NigigMapView {
                 .chars()
                 .take(120)
                 .collect::<String>();
-            let error_msg = format!(
+            self.cache.mark_failed(
-                "HTTP {} for tile z{} x{} y{} (gen {}): {}",
+                tile_key,
-                response.status_code,
+                &format!(
-                tile_key.z,
+                    "http status {} body: {}",
-                tile_key.x,
+                    response.status_code, preview
-                tile_key.y,
+                ),
-                generation,
-                preview
             );
-            log!("NigigMapView: {}", error_msg);
-            self.cache.mark_failed(tile_key, &error_msg);
             self.update_status_text();
             self.redraw(cx);
             return;
         }
 
         let Some(body) = response.get_string_body() else {
-            let error_msg = format!(
+            self.cache.mark_failed(tile_key, "missing utf8 response body");
-                "Missing UTF-8 response body for tile z{} x{} y{} (gen {})",
-                tile_key.z,
-                tile_key.x,
-                tile_key.y,
-                generation
-            );
-            log!("NigigMapView: {}", error_msg);
-            self.cache.mark_failed(tile_key, &error_msg);
             self.update_status_text();
             self.redraw(cx);
             return;
@@ -606,30 +593,13 @@ impl WidgetMatchEvent for NigigMapView {
 
         let current_gen = self.scheduler.current_generation();
         if generation != current_gen {
-            log!(
+            log!("NigigMapView: discarding stale HTTP tile {:?} (gen {} vs {})", tile_key, generation, current_gen);
-                "NigigMapView: discarding stale HTTP tile z{} x{} y{} (gen {} vs current {})",
-                tile_key.z,
-                tile_key.x,
-                tile_key.y,
-                generation,
-                current_gen
-            );
             return;
         }
 
         self.ensure_tile_thread_pool(cx);
         let Some(pool) = self.tile_thread_pool.as_ref() else {
-            let error_msg = format!(
+            log!("NigigMapView: tile thread pool not available");
-                "Tile thread pool not available for tile z{} x{} y{} (gen {})",
-                tile_key.z,
-                tile_key.x,
-                tile_key.y,
-                generation
-            );
-            log!("NigigMapView: {}", error_msg);
-            self.cache.mark_failed(tile_key, &error_msg);
-            self.update_status_text();
-            self.redraw(cx);
             return;
         };
         let sender = self.tile_worker_rx.sender();
@@ -666,20 +636,13 @@ impl WidgetMatchEvent for NigigMapView {
         err: &HttpError,
         _scope: &mut Scope,
     ) {
-        let Some((tile_key, generation)) = self.scheduler.on_http_error(request_id) else {
+        let Some((tile_key, _generation)) = self.scheduler.on_http_error(request_id) else {
-            log!("NigigMapView: received HTTP error for unknown request_id {:?}", request_id);
             return;
         };
-        let error_msg = format!(
+        self.cache.mark_failed(
-            "HTTP request error for tile z{} x{} y{} (gen {}): {:?}",
+            tile_key,
-            tile_key.z,
+            &format!("http request error: {:?}", err),
-            tile_key.x,
-            tile_key.y,
-            generation,
-            err
         );
-        log!("NigigMapView: {}", error_msg);
-        self.cache.mark_failed(tile_key, &error_msg);
         self.update_status_text();
         self.redraw(cx);
     }
@@ -995,7 +958,7 @@ impl NigigMapView {
         }
 
         let target_zoom = self.viewport.request_zoom_level(self.use_local_mbtiles);
-        self.cache.evict(cx, &visible_set, target_zoom);
+        self.cache.evict(&visible_set, target_zoom);
         self.update_status_text();
     }