fix: prevent heartbeat closure errors by leaking closure via forget()

Uses Closure::forget() to intentionally leak the heartbeat closure,
preventing "closure invoked after being dropped" WASM errors when
databases are closed while heartbeat intervals are pending.

The heartbeat_valid flag makes the leaked closure a no-op after
stop_election is called. Trade-off: ~100 bytes memory leak per
database but prevents runtime errors.

Author: npiesco

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "absurder-sql"
-version = "0.1.20"
+version = "0.1.21"
 authors = ["Nicholas G. Piesco"]
 description = "AbsurderSQL - SQLite + IndexedDB that's absurdly better than absurd-sql"
 license = "AGPL-3.0"

Cargo.toml

@@ -73,6 +73,7 @@ async function initDB() {
 
     // Expose db wrapper on window for testing
     window.db = db;
+    window.__db__ = db;  // Test parity with other e2e tests
 
     await db.init();
 
@@ -109,6 +110,9 @@ async function initDB() {
     }, 2000);
 
     status('Ready!');
+
+    // Signal that database is fully initialized for e2e tests
+    window.__db__ = db;
 }
 
 async function runTest() {

examples/vite-app/main.js

@@ -1,6 +1,6 @@
 {
   "name": "@npiesco/absurder-sql",
-  "version": "0.1.20",
+  "version": "0.1.21",
   "description": "High-performance SQLite for browsers with IndexedDB persistence. Export/import databases, multi-tab coordination, dual-mode (browser + native).",
   "type": "module",
   "main": "./pkg/absurder_sql.js",

package.json

@@ -1732,12 +1732,8 @@ impl Drop for Database {
                             );
                         }
                     }
-                    // Drop closure to release Rc references
-                    if manager.heartbeat_closure.take().is_some() {
-                        web_sys::console::log_1(
-                            &format!("DROP: Released heartbeat closure for {}", self.name).into(),
-                        );
-                    }
+                    // Note: heartbeat closure is intentionally leaked (via forget())
+                    // and becomes a no-op when heartbeat_valid is set to false
                 }
             } else {
                 // Manager already borrowed - skip (first DB is cleaning up)

src/lib.rs

@@ -2043,12 +2043,8 @@ impl BlockStorage {
                         );
                     }
                 }
-                // Also drop the closure to release Rc references
-                if manager.heartbeat_closure.take().is_some() {
-                    web_sys::console::log_1(
-                        &format!("[DROP] Released heartbeat closure for {}", self.db_name).into(),
-                    );
-                }
+                // Note: heartbeat closure is intentionally leaked (via forget())
+                // and becomes a no-op when heartbeat_valid is set to false
             }
         } else {
             // Already borrowed (e.g., first DB is still dropping) - skip silently

src/storage/block_storage.rs

@@ -34,9 +34,14 @@ pub struct LeaderElectionManager {
     pub state: Rc<RefCell<LeaderElectionState>>,
     broadcast_channel: Option<BroadcastChannel>,
     pub heartbeat_interval: Option<i32>,
-    pub heartbeat_closure: Option<Closure<dyn FnMut()>>,
+    // NOTE: heartbeat_closure is intentionally leaked via Closure::forget()
+    // to prevent "closure invoked after being dropped" errors from pending ticks.
+    // The heartbeat_valid flag makes the leaked closure a no-op after stop.
     message_listener: Option<Closure<dyn FnMut(web_sys::MessageEvent)>>,
     lease_duration_ms: u64,
+    /// Validity flag - set to false before clearing interval to prevent
+    /// leaked closure from doing any work after stop_election is called
+    heartbeat_valid: Rc<RefCell<bool>>,
 }
 
 impl LeaderElectionManager {
@@ -60,9 +65,9 @@ impl LeaderElectionManager {
             })),
             broadcast_channel: None,
             heartbeat_interval: None,
-            heartbeat_closure: None,
             message_listener: None,
             lease_duration_ms: 1000, // 1 second - fast leader election
+            heartbeat_valid: Rc::new(RefCell::new(false)),
         }
     }
 
@@ -420,8 +425,20 @@ impl LeaderElectionManager {
 
         // Now set up interval for periodic updates
         let state_clone = self.state.clone();
+        let valid_clone = self.heartbeat_valid.clone();
+
+        // Mark heartbeat as valid before starting
+        *self.heartbeat_valid.borrow_mut() = true;
 
         let closure = Closure::wrap(Box::new(move || {
+            // CRITICAL: Check validity FIRST before any other operations
+            // This prevents "closure invoked after being dropped" errors
+            // when a pending setInterval tick fires after stop_election invalidates
+            if !*valid_clone.borrow() {
+                // Heartbeat has been invalidated - don't execute
+                return;
+            }
+
             // Reentrancy guard: skip if heartbeat is already running
             // This prevents "closure invoked recursively" errors from wasm-bindgen
             let already_running = HEARTBEAT_RUNNING.with(|running| {
@@ -497,17 +514,30 @@ impl LeaderElectionManager {
                 )
             })?;
 
-        // CRITICAL: Store closure instead of forgetting it, so it can be properly cleaned up
         self.heartbeat_interval = Some(interval_id);
-        self.heartbeat_closure = Some(closure);
+
+        // CRITICAL: Intentionally leak the closure via forget() to prevent
+        // "closure invoked after being dropped" errors.
+        //
+        // When the manager is dropped:
+        // 1. heartbeat_valid is set to false (closure becomes no-op)
+        // 2. clearInterval is called (stops future scheduling)
+        // 3. But pending callbacks in JS event queue may still fire
+        // 4. Since the closure is leaked (never dropped), those callbacks
+        //    will safely invoke the Rust closure which immediately returns
+        //    due to the validity check.
+        //
+        // Trade-off: Small memory leak (~100 bytes per database) but prevents
+        // runtime errors. Databases are typically long-lived so this is acceptable.
+        closure.forget();
 
         Ok(())
     }
 
     /// Stop leader election (e.g., when tab is closing)
     pub async fn stop_election(&mut self) -> Result<(), DatabaseError> {
         // CRITICAL: Check if already stopped (idempotent)
-        if self.heartbeat_interval.is_none() && self.heartbeat_closure.is_none() {
+        if self.heartbeat_interval.is_none() && !*self.heartbeat_valid.borrow() {
             web_sys::console::log_1(&"[STOP] Already stopped - skipping".into());
             return Ok(());
         }
@@ -524,7 +554,13 @@ impl LeaderElectionManager {
             was_leader
         );
 
-        // CRITICAL: Clear interval and closure FIRST to release Rc references
+        // CRITICAL: Invalidate heartbeat FIRST, before clearing interval
+        // This ensures any pending setInterval tick will bail out immediately
+        // and won't try to execute with freed resources
+        *self.heartbeat_valid.borrow_mut() = false;
+        web_sys::console::log_1(&format!("[STOP] Invalidated heartbeat for {}", db_name).into());
+
+        // Now clear interval (closure is intentionally leaked, no need to drop)
         if let Some(interval_id) = self.heartbeat_interval.take() {
             web_sys::console::log_1(
                 &format!("[STOP] Clearing interval {} for {}", interval_id, db_name).into(),
@@ -534,13 +570,6 @@ impl LeaderElectionManager {
             }
         }
 
-        // Drop the closure to release any Rc<RefCell<State>> references
-        if let Some(_closure) = self.heartbeat_closure.take() {
-            web_sys::console::log_1(
-                &format!("[STOP] Dropped heartbeat closure for {}", db_name).into(),
-            );
-        }
-
         // CRITICAL: Close the BroadcastChannel to prevent test interference
         if let Some(channel) = self.broadcast_channel.take() {
             channel.close();
@@ -726,3 +755,40 @@ impl LeaderElectionManager {
         state.last_heartbeat
     }
 }
+
+/// Drop implementation to prevent "closure invoked after being dropped" errors
+///
+/// CRITICAL: Invalidate heartbeat_valid BEFORE clearing interval.
+/// The heartbeat closure is intentionally leaked via forget() so it's never dropped.
+/// Any pending setInterval ticks will safely invoke the leaked closure which
+/// immediately returns due to the validity check.
+impl Drop for LeaderElectionManager {
+    fn drop(&mut self) {
+        // CRITICAL: Invalidate heartbeat FIRST - leaked closure will become no-op
+        *self.heartbeat_valid.borrow_mut() = false;
+
+        // Clear heartbeat interval (stops future scheduling)
+        if let Some(interval_id) = self.heartbeat_interval.take() {
+            if let Some(window) = web_sys::window() {
+                window.clear_interval_with_handle(interval_id);
+                log::debug!(
+                    "LeaderElectionManager::drop() - Cleared heartbeat interval {}",
+                    interval_id
+                );
+            }
+        }
+
+        // Close broadcast channel
+        if let Some(channel) = self.broadcast_channel.take() {
+            channel.close();
+            log::debug!("LeaderElectionManager::drop() - Closed BroadcastChannel");
+        }
+
+        // Note: heartbeat closure is intentionally leaked (never dropped)
+        // message_listener will be dropped here
+        log::debug!(
+            "LeaderElectionManager::drop() - Cleanup complete for {}",
+            self.state.borrow().db_name
+        );
+    }
+}

src/storage/leader_election.rs

@@ -0,0 +1,134 @@
+/**
+ * Test for "closure invoked after being dropped" error
+ *
+ * Root cause: LeaderElectionManager has no Drop impl, so when the manager
+ * is dropped, clearInterval is never called. The setInterval keeps firing
+ * and invokes a freed closure.
+ *
+ * Expected behavior: No errors after database is closed/dropped
+ * Current behavior: "closure invoked recursively or after being dropped" errors
+ */
+import { test, expect } from '@playwright/test';
+
+test.describe('Heartbeat Drop-After-Freed Bug', () => {
+  test.beforeEach(async ({ page }) => {
+    // Navigate to vite app
+    await page.goto('http://localhost:3000');
+
+    // Wait for database to be ready (window.__db__ exposed after init)
+    await page.waitForFunction(() => window.__db__ !== undefined, { timeout: 30000 });
+  });
+
+  test('should NOT error when database is dropped while heartbeat is running', async ({ page }) => {
+    const errors = [];
+
+    // Capture all closure-related errors
+    page.on('console', msg => {
+      if (msg.type() === 'error') {
+        const text = msg.text();
+        if (text.includes('closure invoked') || text.includes('after being dropped')) {
+          errors.push({ type: 'console', text });
+        }
+      }
+    });
+
+    page.on('pageerror', err => {
+      if (err.message.includes('closure invoked') || err.message.includes('after being dropped')) {
+        errors.push({ type: 'pageerror', text: err.message });
+      }
+    });
+
+    console.log('[TEST] Database initialized, creating additional databases...');
+
+    // Create and immediately destroy multiple databases to trigger the bug
+    const result = await page.evaluate(async () => {
+      const results = [];
+      const Database = window.Database;
+
+      for (let i = 0; i < 3; i++) {
+        try {
+          // Create a new database (starts heartbeat via leader election)
+          const db = await Database.newDatabase(`drop_test_${i}_${Date.now()}`);
+          results.push({ step: `create_${i}`, success: true });
+
+          // Do a quick operation to ensure it's fully initialized
+          await db.execute('SELECT 1');
+          results.push({ step: `query_${i}`, success: true });
+
+          // Wait a bit to let heartbeat establish
+          await new Promise(r => setTimeout(r, 1500));
+
+          // Close/drop the database (should clearInterval, but doesn't!)
+          await db.close();
+          results.push({ step: `close_${i}`, success: true });
+
+        } catch (e) {
+          results.push({ step: `error_${i}`, success: false, error: e.message });
+        }
+      }
+
+      return results;
+    });
+
+    console.log('[TEST] Database operations:', JSON.stringify(result, null, 2));
+
+    // Wait 3 seconds for any post-drop interval fires
+    // Heartbeat is 1 second, so 3 seconds = 3 potential ghost invocations per dropped db
+    console.log('[TEST] Waiting 3 seconds for ghost heartbeat invocations...');
+    await page.waitForTimeout(3000);
+
+    console.log('[TEST] Errors captured:', errors.length);
+    if (errors.length > 0) {
+      console.log('[TEST] Error details:', JSON.stringify(errors, null, 2));
+    }
+
+    // ASSERTION: No "after being dropped" errors should occur
+    // This test will FAIL until the Drop impl is added to LeaderElectionManager
+    expect(errors.length).toBe(0);
+  });
+
+  test('rapid create-destroy cycle should not leak intervals', async ({ page }) => {
+    const errors = [];
+
+    page.on('console', msg => {
+      if (msg.type() === 'error') {
+        errors.push(msg.text());
+      }
+    });
+
+    page.on('pageerror', err => {
+      errors.push(err.message);
+    });
+
+    // Rapid create-destroy cycle
+    const result = await page.evaluate(async () => {
+      const Database = window.Database;
+      const ops = [];
+
+      // Rapid fire: create and immediately close 10 databases
+      for (let i = 0; i < 10; i++) {
+        const db = await Database.newDatabase(`rapid_${i}_${Date.now()}`);
+        await db.execute('SELECT 1');
+        await db.close();
+        ops.push(i);
+      }
+
+      return ops;
+    });
+
+    console.log('[TEST] Completed rapid cycle:', result.length, 'databases');
+
+    // Wait for potential ghost intervals (10 dbs × 1s heartbeat × 5 seconds = many potential errors)
+    await page.waitForTimeout(5000);
+
+    // Filter for closure errors specifically
+    const closureErrors = errors.filter(e =>
+      e.includes('closure invoked') || e.includes('after being dropped')
+    );
+
+    console.log('[TEST] Closure errors after rapid cycle:', closureErrors.length);
+
+    // This will FAIL until Drop impl is added
+    expect(closureErrors.length).toBe(0);
+  });
+});

tests/e2e/heartbeat-drop-after-freed.spec.js

@@ -5,12 +5,12 @@
  * invoked recursively if the previous tick hasn't completed. This test
  * verifies that the reentrancy guard prevents "closure invoked recursively" errors.
  */
-const { test, expect } = require('@playwright/test');
+import { test, expect } from '@playwright/test';
 
 test.describe('Heartbeat Reentrancy Guard', () => {
   test.beforeEach(async ({ page }) => {
     // Navigate to the vite example app
-    await page.goto('http://localhost:5173');
+    await page.goto('http://localhost:3000');
 
     // Wait for the app to load and database to initialize
     await page.waitForFunction(() => {