crude memory leak test

Author: alshdavid

crates/ion/src/values/js_external.rs

@@ -13,7 +13,7 @@ use crate::values::ToJsValue;
 pub struct JsExternal<T> {
     pub(crate) value: Value,
     pub(crate) env: Env,
-    ptr: *mut c_void,
+    data: *mut (*mut c_void, RefCounter),
     ref_count: RefCounter,
     _data: PhantomData<T>,
 }
@@ -23,12 +23,14 @@ impl<T> JsExternal<T> {
         env: &Env,
         data: T,
     ) -> crate::Result<Self> {
-        let ptr = Box::into_raw(Box::new(data)) as *mut c_void;
         let scope = &mut env.scope();
 
         // One for Rust, One for JavaScript
         let ref_count = RefCounter::new(2);
 
+        // Convert the data into a pointer
+        let ptr = Box::into_raw(Box::new(data)) as *mut c_void;
+
         // Store both the data pointer AND the RefCounter in the V8 External
         let external_data = Box::into_raw(Box::new((ptr, ref_count.clone())));
         let value = v8::External::new(scope, external_data as _);
@@ -38,23 +40,25 @@ impl<T> JsExternal<T> {
             move || {
                 if ref_count.dec() {
                     // Clean up both the data and the external_data tuple
-                    drop(unsafe { Box::from_raw(ptr as *mut T) });
-                    drop(unsafe { Box::from_raw(external_data) });
+                    let (ptr, ref_counter) = *unsafe { Box::from_raw(external_data) };
+                    let data = unsafe { Box::from_raw(ptr as *mut T) };
+                    drop(data);
+                    drop(ref_counter);
                 }
             }
         });
 
         Ok(Self {
             value: sys::v8_from_value(value),
             env: env.clone(),
-            ptr,
+            data: external_data,
             ref_count,
             _data: Default::default(),
         })
     }
 
     pub fn as_inner(&self) -> crate::Result<&T> {
-        let data = unsafe { &*(self.ptr as *mut T) };
+        let data = unsafe { &*((*self.data).0 as *mut T) };
         Ok(data)
     }
 }
@@ -65,7 +69,7 @@ impl<T> Clone for JsExternal<T> {
         Self {
             value: self.value,
             env: self.env.clone(),
-            ptr: self.ptr,
+            data: self.data,
             ref_count: self.ref_count.clone(),
             _data: self._data,
         }
@@ -75,7 +79,10 @@ impl<T> Clone for JsExternal<T> {
 impl<T> Drop for JsExternal<T> {
     fn drop(&mut self) {
         if self.ref_count.dec() {
-            drop(unsafe { Box::from_raw(self.ptr as *mut T) });
+            let (ptr, ref_counter) = *unsafe { Box::from_raw(self.data) };
+            let data = unsafe { Box::from_raw(ptr as *mut T) };
+            drop(data);
+            drop(ref_counter);
         }
     }
 }
@@ -98,16 +105,16 @@ impl<T> FromJsValue for JsExternal<T> {
         value: Value,
     ) -> crate::Result<Self> {
         let external = value.cast::<v8::External>();
-        let external_data_ptr = external.value() as *const (*mut c_void, RefCounter);
-        let (ptr, ref_count) = unsafe { &*external_data_ptr };
+        let external_data_ptr = external.value() as *mut (*mut c_void, RefCounter);
+        let (_, ref_count) = unsafe { &*external_data_ptr };
 
         // Increment the original RefCounter instead of creating a new one
         ref_count.inc();
 
         Ok(Self {
             value,
             env: env.clone(),
-            ptr: *ptr,
+            data: external_data_ptr,
             ref_count: ref_count.clone(),
             _data: Default::default(),
         })

examples/src/_utils/memory_blob.rs

@@ -0,0 +1,4 @@
+/// Create a 100kb memory allocation
+pub fn blob_100kb() -> Vec<u8> {
+    vec![0u8; 100 * 1024]
+}

examples/src/_utils/memory_usage.rs

@@ -31,7 +31,7 @@ impl MemoryUsageCounter {
         } else if current == *previous {
             0
         } else {
-            -(current - *previous)
+            current - *previous
         };
 
         (*previous) = current;

examples/src/_utils/mod.rs

@@ -1 +1,2 @@
+pub mod memory_blob;
 pub mod memory_usage;

examples/src/main.rs

@@ -10,6 +10,7 @@ mod eval;
 mod external_value;
 mod http_server;
 mod memory_usage_context;
+mod memory_usage_external_value;
 mod memory_usage_module;
 mod memory_usage_tsfn;
 mod memory_usage_value;
@@ -55,6 +56,7 @@ fn main() -> anyhow::Result<()> {
         "memory_usage_module" => memory_usage_module::main(),
         "memory_usage_value" => memory_usage_value::main(),
         "external_value" => external_value::main(),
+        "memory_usage_external_value" => memory_usage_external_value::main(),
         _ => Err(anyhow::anyhow!("No example for: \"{}\"", example)),
     }
 }

examples/src/memory_usage_external_value/memory_usage_external_value.test.ts

@@ -0,0 +1,14 @@
+import { executeMemoryTest } from "../../test-utils/memory-leak.ts";
+import { assertLessOrEqual } from "jsr:@std/assert@^1";
+
+Deno.test("memory_usage_external_value", async () => {
+    const report = await executeMemoryTest("memory_usage_external_value");
+
+    assertLessOrEqual(
+        report.overAllMemoryTrendPerSample,
+        5,
+        `Memory usage in the last quarter increase at an average of ${report.overAllMemoryTrendPerSample.toFixed(
+            0
+        )}kb per sample`
+    );
+});

examples/src/memory_usage_external_value/mod.rs

@@ -0,0 +1,50 @@
+use ion::*;
+
+use crate::_utils::memory_blob::blob_100kb;
+use crate::_utils::memory_usage::MemoryUsageCounter;
+
+pub fn main() -> anyhow::Result<()> {
+    let memu = MemoryUsageCounter::default();
+    println!("{}", memu.kilobytes().json());
+
+    let runtime = JsRuntime::initialize_once(JsRuntimeOptions::debug(JsRuntimeOptions {
+        ..Default::default()
+    }))?;
+
+    let worker = runtime.spawn_worker(JsWorkerOptions::default())?;
+
+    for _ in 0..10 {
+        worker.run_garbage_collection_for_testing()?;
+
+        let ctx = worker.create_context()?;
+        println!("{}", memu.kilobytes().json());
+
+        for _ in 0..100 {
+            println!("{}", memu.kilobytes().json());
+            let data = blob_100kb();
+
+            ctx.exec_blocking(move |env| {
+                let external_js = JsExternal::new(env, data)?;
+                env.global_this()?
+                    .set_named_property("external", external_js)?;
+
+                Ok(())
+            })?;
+
+            // TODO
+            // Currently GC only seems to fire when the context is dropped
+            ctx.exec_blocking(|env| env.global_this()?.delete_named_property("external"))?;
+            worker.run_garbage_collection_for_testing()?;
+            // TODO-END
+
+            println!("{}", memu.kilobytes().json());
+        }
+
+        println!("{}", memu.kilobytes().json());
+    }
+
+    worker.run_garbage_collection_for_testing()?;
+    println!("{}", memu.kilobytes().json());
+
+    Ok(())
+}

examples/test-utils/memory-leak.ts

@@ -0,0 +1,138 @@
+// Credit to ChatGPT
+
+import { executeExampleStream } from './run_test.ts'
+
+type ProcessMessage = {
+  value: number,
+  change: number,
+}
+
+export type MemoryUsageReport = {
+  overAllMemoryTrendPerSample: number,
+  firstHalfAverage: number,
+  secondHalfAverage: number,
+  lastQuarterTrendPerSample: number,
+  lastQuarterStdDev: number,
+  memoryIncreaseFromFirstToSecondHalf: number,
+  memoryIncreaseFromFirstToSecondHalfPercent: number,
+}
+
+export async function executeMemoryTest(testName: string,
+  args: string[] = [],
+  env: Record<string, string> = {}): Promise<MemoryUsageReport> {
+  const { done, stdout } = await executeExampleStream(testName, args, env)
+
+  const samples: number[] = [];
+  const WARMUP_SAMPLES = 5; // Skip initial samples during warmup
+
+  for await (const recordStr of stdout) {
+    const record: ProcessMessage = JSON.parse(recordStr + '\n')
+    samples.push(record.value);
+    // console.log(`Memory: ${record.value}KB (${record.change >= 0 ? '+' : ''}${record.change}KB)`);
+  }
+
+  await done
+
+  // Analysis after process completes
+  if (samples.length < 15) {
+    throw new Error('Not enough samples collected for reliable leak detection (need at least 15)');
+  }
+
+  // Skip warmup phase and analyze the rest
+  const steadyStateSamples = samples.slice(WARMUP_SAMPLES);
+
+  // Strategy 1: Check if memory trend is consistently upward
+  const overallTrend = calculateTrend(steadyStateSamples);
+  // console.log(`\nOverall memory trend: ${overallTrend > 0 ? '+' : ''}${overallTrend.toFixed(2)}KB per sample`);
+
+  // Strategy 2: Compare first half vs second half
+  const midpoint = Math.floor(steadyStateSamples.length / 2);
+  const firstHalf = steadyStateSamples.slice(0, midpoint);
+  const secondHalf = steadyStateSamples.slice(midpoint);
+
+  const firstHalfAvg = average(firstHalf);
+  const secondHalfAvg = average(secondHalf);
+  const halfDiff = secondHalfAvg - firstHalfAvg;
+
+  // console.log(`First half average: ${firstHalfAvg.toFixed(2)}KB`);
+  // console.log(`Second half average: ${secondHalfAvg.toFixed(2)}KB`);
+  // console.log(`Difference: ${halfDiff >= 0 ? '+' : ''}${halfDiff.toFixed(2)}KB`);
+
+  // Strategy 3: Check for stabilization
+  const lastQuarter = steadyStateSamples.slice(-Math.floor(steadyStateSamples.length / 4));
+  const lastQuarterTrend = calculateTrend(lastQuarter);
+  const lastQuarterStdDev = standardDeviation(lastQuarter);
+
+  // console.log(`Last quarter trend: ${lastQuarterTrend > 0 ? '+' : ''}${lastQuarterTrend.toFixed(2)}KB per sample`);
+  // console.log(`Last quarter std dev: ${lastQuarterStdDev.toFixed(2)}KB`);
+
+  // Detect leak based on multiple signals
+  // const leakSignals: string[] = [];
+
+  // Signal 1: Strong upward trend overall (>512KB per sample)
+  // if (overallTrend > 512) {
+  //   leakSignals.push(`Strong upward trend: ${overallTrend.toFixed(2)}KB per sample`);
+  // }
+
+  // Signal 2: Second half significantly higher than first (>20MB or >20% increase)
+  const percentIncrease = (halfDiff / firstHalfAvg) * 100;
+  // if (halfDiff > 20480 || percentIncrease > 20) {
+  //   leakSignals.push(`Memory increased ${halfDiff.toFixed(2)}KB (${percentIncrease.toFixed(1)}%) from first to second half`);
+  // }
+
+  // Signal 3: Memory still climbing at the end (trend in last quarter >307KB)
+  // if (lastQuarterTrend > 307) {
+  //   leakSignals.push(`Memory still climbing at end: ${lastQuarterTrend.toFixed(2)}KB per sample in last quarter`);
+  // }
+
+  // Signal 4: High variance in last quarter suggests unstable memory (may indicate leak)
+  // if (lastQuarterStdDev > 15360 && lastQuarterTrend > 205) {
+  //   leakSignals.push(`High memory variance with upward trend: ${lastQuarterStdDev.toFixed(2)}KB std dev`);
+  // }
+
+  // if (leakSignals.length >= 2) {
+    // throw new Error(
+    //   `Memory leak detected (${leakSignals.length} signals):\n` +
+    //   leakSignals.map(s => `  - ${s}`).join('\n')
+    // );
+  // }
+
+  // if (leakSignals.length === 1) {
+  //   console.warn(`\n⚠ Potential leak indicator: ${leakSignals[0]}`);
+  // }
+
+  // console.log('\n✓ No significant memory leak detected');
+
+  return {
+    overAllMemoryTrendPerSample: overallTrend,
+    firstHalfAverage: firstHalfAvg,
+    secondHalfAverage: secondHalfAvg,
+    lastQuarterTrendPerSample: lastQuarterTrend,
+    lastQuarterStdDev: lastQuarterStdDev,
+    memoryIncreaseFromFirstToSecondHalf: halfDiff,
+    memoryIncreaseFromFirstToSecondHalfPercent: percentIncrease,
+  }
+};
+
+function average(samples: number[]): number {
+  return samples.reduce((a, b) => a + b, 0) / samples.length;
+}
+
+function standardDeviation(samples: number[]): number {
+  const avg = average(samples);
+  const squareDiffs = samples.map(value => Math.pow(value - avg, 2));
+  return Math.sqrt(average(squareDiffs));
+}
+
+function calculateTrend(samples: number[]): number {
+  const n = samples.length;
+  const indices = Array.from({ length: n }, (_, i) => i);
+
+  const sumX = indices.reduce((a, b) => a + b, 0);
+  const sumY = samples.reduce((a, b) => a + b, 0);
+  const sumXY = indices.reduce((sum, x, i) => sum + x * samples[i], 0);
+  const sumX2 = indices.reduce((sum, x) => sum + x * x, 0);
+
+  const slope = (n * sumXY - sumX * sumY) / (n * sumX2 - sumX * sumX);
+  return slope;
+}