From fca5ef44cc998a06a11a9db80e41d7698721b9f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Jan 2026 18:50:54 +0000 Subject: [PATCH] Fix memory leaks in exception handling and task join Fix three memory management issues: 1. try/finally: When finally block throws/returns, the saved exception and return values were lost without being released. Now properly retains saved values before finally executes and releases them appropriately based on control flow outcome. 2. defer stack: When a deferred call throws while an exception is pending, the original exception was lost without being released. Now properly retains the saved exception and releases it if the defer throws a new exception. 3. task join: When joining a task that threw an exception, the exception_state was copied without retaining the exception_value, leading to incorrect refcounting. Now retains the exception value when copying from task to caller context. https://claude.ai/code/session_01DfpaWxNKzvp8vMc6sEhLjC --- .../interpreter/builtins/concurrency.c | 2 ++ src/backends/interpreter/runtime/context.c | 8 ++++++++ src/backends/interpreter/runtime/statements.c | 11 ++++++++++ tests/memory/defer_exception_leak.hml | 20 +++++++++++++++++++ tests/memory/finally_exception_leak.hml | 19 ++++++++++++++++++ tests/memory/join_exception_refcount.hml | 16 +++++++++++++++ 6 files changed, 76 insertions(+) create mode 100644 tests/memory/defer_exception_leak.hml create mode 100644 tests/memory/finally_exception_leak.hml create mode 100644 tests/memory/join_exception_refcount.hml diff --git a/src/backends/interpreter/builtins/concurrency.c b/src/backends/interpreter/builtins/concurrency.c index 74fe77cf..6b6f462e 100644 --- a/src/backends/interpreter/builtins/concurrency.c +++ b/src/backends/interpreter/builtins/concurrency.c @@ -187,7 +187,9 @@ Value builtin_join(Value *args, int num_args, ExecutionContext *ctx) { // Check if task threw an exception if (task->ctx->exception_state.is_throwing) { // Re-throw the exception in the current context + // Retain the exception value since both task and caller will release it ctx->exception_state = task->ctx->exception_state; + VALUE_RETAIN(ctx->exception_state.exception_value); pthread_mutex_unlock((pthread_mutex_t*)task->task_mutex); return val_null(); } diff --git a/src/backends/interpreter/runtime/context.c b/src/backends/interpreter/runtime/context.c index 04c78317..620d90b6 100644 --- a/src/backends/interpreter/runtime/context.c +++ b/src/backends/interpreter/runtime/context.c @@ -447,6 +447,9 @@ void defer_stack_execute(DeferStack *stack, ExecutionContext *ctx) { int was_throwing = ctx->exception_state.is_throwing; Value saved_exception = ctx->exception_state.exception_value; + // Retain saved exception to prevent it from being freed during defer execution + VALUE_RETAIN(saved_exception); + // Temporarily clear exception state to allow defer to run ctx->exception_state.is_throwing = 0; @@ -458,6 +461,11 @@ void defer_stack_execute(DeferStack *stack, ExecutionContext *ctx) { if (!ctx->exception_state.is_throwing) { ctx->exception_state.is_throwing = was_throwing; ctx->exception_state.exception_value = saved_exception; + // Release our extra reference since we're restoring the value + VALUE_RELEASE(saved_exception); + } else { + // Defer threw a new exception - release saved exception since it won't be used + VALUE_RELEASE(saved_exception); } // Clean up the deferred expression and release environment diff --git a/src/backends/interpreter/runtime/statements.c b/src/backends/interpreter/runtime/statements.c index cb32b8d1..9d0e743a 100644 --- a/src/backends/interpreter/runtime/statements.c +++ b/src/backends/interpreter/runtime/statements.c @@ -731,6 +731,10 @@ void eval_stmt(Stmt *stmt, Environment *env, ExecutionContext *ctx) { int was_breaking = ctx->loop_state.is_breaking; int was_continuing = ctx->loop_state.is_continuing; + // Retain saved values to prevent them from being freed while we execute finally + VALUE_RETAIN(saved_return); + VALUE_RETAIN(saved_exception); + // Clear states before finally ctx->return_state.is_returning = 0; ctx->exception_state.is_throwing = 0; @@ -749,6 +753,13 @@ void eval_stmt(Stmt *stmt, Environment *env, ExecutionContext *ctx) { ctx->exception_state.exception_value = saved_exception; ctx->loop_state.is_breaking = was_breaking; ctx->loop_state.is_continuing = was_continuing; + // Release our extra references since we're restoring the values + VALUE_RELEASE(saved_return); + VALUE_RELEASE(saved_exception); + } else { + // Finally changed control flow - release saved values since they won't be used + VALUE_RELEASE(saved_return); + VALUE_RELEASE(saved_exception); } } break; diff --git a/tests/memory/defer_exception_leak.hml b/tests/memory/defer_exception_leak.hml new file mode 100644 index 00000000..ff1faa23 --- /dev/null +++ b/tests/memory/defer_exception_leak.hml @@ -0,0 +1,20 @@ +// Test for memory leak when defer throws while exception is pending +// Previously, the original exception would leak when defer threw + +fn throw_error() { + throw "defer exception"; +} + +fn test_defer_throws() { + defer throw_error(); + throw "original exception"; +} + +// Test the scenario +try { + test_defer_throws(); +} catch (e) { + print("Caught: " + e); +} + +print("Test passed - no memory leak"); diff --git a/tests/memory/finally_exception_leak.hml b/tests/memory/finally_exception_leak.hml new file mode 100644 index 00000000..60025bd9 --- /dev/null +++ b/tests/memory/finally_exception_leak.hml @@ -0,0 +1,19 @@ +// Test for memory leak when finally block throws a new exception +// Previously, the original exception would leak when finally threw + +fn test_finally_throws() { + try { + throw "original exception"; + } finally { + throw "finally exception"; + } +} + +// Test the scenario +try { + test_finally_throws(); +} catch (e) { + print("Caught: " + e); +} + +print("Test passed - no memory leak"); diff --git a/tests/memory/join_exception_refcount.hml b/tests/memory/join_exception_refcount.hml new file mode 100644 index 00000000..6c447306 --- /dev/null +++ b/tests/memory/join_exception_refcount.hml @@ -0,0 +1,16 @@ +// Test for refcount mismatch when joining a task that threw an exception +// Previously, the exception value was copied without retaining + +async fn throws_error() { + throw "task exception"; +} + +// Test the scenario +try { + let task = spawn(throws_error); + join(task); +} catch (e) { + print("Caught: " + e); +} + +print("Test passed - no refcount mismatch");