gh-19: Fix race condition caused by env_get_entry.

Author: python-processing-unit

src/env.c

@@ -22,6 +22,67 @@
 #define strdup _strdup
 #endif
 
+/* ================================================================== */
+/*  Thread-local snapshots for env_get_entry                           */
+/* ================================================================== */
+
+// env_get_entry historically returned a raw pointer into Env storage.
+// With the namespace buffer active, that pointer could be invalidated
+// immediately after returning (e.g. via realloc/value_free on the
+// prepare thread), causing use-after-free crashes.
+//
+// Fix: return a per-thread snapshot copy of the entry.
+//
+// NOTE: The returned pointer is valid until the calling thread makes
+// enough subsequent env_get_entry calls to wrap this ring buffer.
+
+#ifndef ENV_ENTRY_SNAPSHOT_RING
+#define ENV_ENTRY_SNAPSHOT_RING 32
+#endif
+
+static _Thread_local EnvEntry g_entry_snaps[ENV_ENTRY_SNAPSHOT_RING];
+static _Thread_local size_t g_entry_snap_idx = 0;
+
+static void env_entry_snap_clear(EnvEntry* e) {
+    if (!e) return;
+    if (e->name) {
+        free(e->name);
+        e->name = NULL;
+    }
+    if (e->alias_target) {
+        free(e->alias_target);
+        e->alias_target = NULL;
+    }
+    value_free(e->value);
+    e->value = value_null();
+    e->decl_type = TYPE_UNKNOWN;
+    e->initialized = false;
+    e->frozen = false;
+    e->permafrozen = false;
+}
+
+static EnvEntry* env_entry_snap_alloc(void) {
+    EnvEntry* slot = &g_entry_snaps[g_entry_snap_idx++ % ENV_ENTRY_SNAPSHOT_RING];
+    env_entry_snap_clear(slot);
+    return slot;
+}
+
+static void env_entry_snap_from_raw(EnvEntry* dst, const EnvEntry* src) {
+    if (!dst) return;
+    if (!src) {
+        // leave dst cleared
+        return;
+    }
+
+    dst->name = src->name ? strdup(src->name) : NULL;
+    dst->decl_type = src->decl_type;
+    dst->initialized = src->initialized;
+    dst->frozen = src->frozen;
+    dst->permafrozen = src->permafrozen;
+    dst->alias_target = src->alias_target ? strdup(src->alias_target) : NULL;
+    dst->value = value_copy(src->value);
+}
+
 /* ================================================================== */
 /*  Helpers                                                            */
 /* ================================================================== */
@@ -345,13 +406,27 @@ int env_permafreeze(Env* env, const char* name) {
 /* ================================================================== */
 
 EnvEntry* env_get_entry(Env* env, const char* name) {
+    EnvEntry* snap = env_entry_snap_alloc();
     if (ns_buffer_active()) {
         ns_buffer_read_lock(name);
         EnvEntry* entry = env_get_entry_raw(env, name);
+        env_entry_snap_from_raw(snap, entry);
         ns_buffer_read_unlock();
-        return entry;
+        if (!entry) {
+            env_entry_snap_clear(snap);
+            return NULL;
+        }
+        return snap;
+    }
+
+    EnvEntry* entry = env_get_entry_raw(env, name);
+    env_entry_snap_from_raw(snap, entry);
+    if (!entry) {
+        // Clear to a canonical empty state and return NULL for not-found.
+        env_entry_snap_clear(snap);
+        return NULL;
     }
-    return env_get_entry_raw(env, name);
+    return snap;
 }
 
 bool env_get(Env* env, const char* name, Value* out_value,

src/env.h

@@ -29,8 +29,11 @@ bool env_assign(Env* env, const char* name, Value value, DeclType type, bool dec
 bool env_get(Env* env, const char* name, Value* out_value, DeclType* out_type, bool* out_initialized);
 bool env_delete(Env* env, const char* name);
 bool env_exists(Env* env, const char* name);
-// Return pointer to the EnvEntry for the given name, searching parents.
-// Caller must NOT free the returned pointer. Returns NULL if not found.
+// Return a per-thread snapshot of the EnvEntry for the given name, searching parents.
+// The snapshot is safe to read after the function returns even when the
+// namespace buffer is active (it does not point into Env storage).
+// Caller must NOT free the returned pointer.
+// Returns NULL if not found.
 EnvEntry* env_get_entry(Env* env, const char* name);
 
 // Create or update an alias (pointer) binding: `name` will become an alias to `target_name`.

src/interpreter.c

@@ -1452,21 +1452,34 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
         return make_error("Indexed assignment base must be an identifier", stmt_line, stmt_col);
     }
 
-    EnvEntry* entry = env_get_entry(env, walker->as.ident);
-    if (!entry) {
+    const char* base_name = walker->as.ident;
+    Value base_val = value_null();
+    DeclType base_type = TYPE_UNKNOWN;
+    bool base_initialized = false;
+
+    if (!env_get(env, base_name, &base_val, &base_type, &base_initialized)) {
         char buf[256];
-        snprintf(buf, sizeof(buf), "Cannot assign to undeclared identifier '%s'", walker->as.ident);
+        snprintf(buf, sizeof(buf), "Cannot assign to undeclared identifier '%s'", base_name);
         return make_error(buf, stmt_line, stmt_col);
     }
 
-    // If uninitialized, default to map (matches previous indexed-assignment behavior).
-    if (!entry->initialized) {
-        entry->value = value_map_new();
-        entry->initialized = true;
+    // If uninitialized (or NULL), default to MAP (matches previous behaviour),
+    // and persist that into the environment via env_assign.
+    if (!base_initialized || base_val.type == VAL_NULL) {
+        Value nm = value_map_new();
+        if (!env_assign(env, base_name, nm, TYPE_UNKNOWN, false)) {
+            value_free(nm);
+            value_free(base_val);
+            return make_error("Cannot assign to identifier (frozen?)", stmt_line, stmt_col);
+        }
+        value_free(base_val);
+        base_val = nm;
+        base_initialized = true;
     }
 
     Expr** nodes = malloc(sizeof(Expr*) * (chain_len ? chain_len : 1));
     if (!nodes) {
+        value_free(base_val);
         return make_error("Out of memory", stmt_line, stmt_col);
     }
 
@@ -1476,15 +1489,16 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
         walker = walker->as.index.target;
     }
 
-    Value* cur = &entry->value;
+    ExecResult out;
+    Value* cur = &base_val;
 
     // Process from innermost -> outermost
     for (int ni = (int)chain_len - 1; ni >= 0; ni--) {
         Expr* node = nodes[ni];
         ExprList* indices = &node->as.index.indices;
         if (indices->count == 0) {
-            free(nodes);
-            return make_error("Empty index list", node->line, node->column);
+            out = make_error("Empty index list", node->line, node->column);
+            goto cleanup;
         }
 
         // Auto-promote NULL to MAP when assigning through indexes.
@@ -1496,37 +1510,37 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
             Tensor* t = cur->as.tns;
             // Lvalue assignment only supports full integer indexing (no ranges/wildcards).
             if (indices->count != t->ndim) {
-                free(nodes);
-                return make_error("Cannot assign through tensor slice", node->line, node->column);
+                out = make_error("Cannot assign through tensor slice", node->line, node->column);
+                goto cleanup;
             }
 
             size_t* idxs0 = malloc(sizeof(size_t) * indices->count);
             if (!idxs0) {
-                free(nodes);
-                return make_error("Out of memory", stmt_line, stmt_col);
+                out = make_error("Out of memory", stmt_line, stmt_col);
+                goto cleanup;
             }
 
             for (size_t i = 0; i < indices->count; i++) {
                 Expr* it = indices->items[i];
                 if (it->type == EXPR_WILDCARD || it->type == EXPR_RANGE) {
                     free(idxs0);
-                    free(nodes);
-                    return make_error("Cannot assign using ranges or wildcards", it->line, it->column);
+                    out = make_error("Cannot assign using ranges or wildcards", it->line, it->column);
+                    goto cleanup;
                 }
 
                 Value vi = eval_expr(interp, it, env);
                 if (interp->error) {
                     ExecResult err = make_error(interp->error, interp->error_line, interp->error_col);
                     clear_error(interp);
                     free(idxs0);
-                    free(nodes);
-                    return err;
+                    out = err;
+                    goto cleanup;
                 }
                 if (vi.type != VAL_INT) {
                     value_free(vi);
                     free(idxs0);
-                    free(nodes);
-                    return make_error("Index expression must evaluate to INT", it->line, it->column);
+                    out = make_error("Index expression must evaluate to INT", it->line, it->column);
+                    goto cleanup;
                 }
 
                 int64_t v = vi.as.i;
@@ -1535,8 +1549,8 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
                 if (v < 1 || v > dim) {
                     value_free(vi);
                     free(idxs0);
-                    free(nodes);
-                    return make_error("Index out of range", it->line, it->column);
+                    out = make_error("Index out of range", it->line, it->column);
+                    goto cleanup;
                 }
                 idxs0[i] = (size_t)(v - 1);
                 value_free(vi);
@@ -1545,8 +1559,8 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
             Value* elem = value_tns_get_ptr(*cur, idxs0, indices->count);
             free(idxs0);
             if (!elem) {
-                free(nodes);
-                return make_error("Index out of range", node->line, node->column);
+                out = make_error("Index out of range", node->line, node->column);
+                goto cleanup;
             }
             cur = elem;
             continue;
@@ -1559,13 +1573,13 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
                 if (interp->error) {
                     ExecResult err = make_error(interp->error, interp->error_line, interp->error_col);
                     clear_error(interp);
-                    free(nodes);
-                    return err;
+                    out = err;
+                    goto cleanup;
                 }
                 if (!(key.type == VAL_INT || key.type == VAL_STR || key.type == VAL_FLT)) {
                     value_free(key);
-                    free(nodes);
-                    return make_error("Map index must be INT, FLT or STR", it->line, it->column);
+                    out = make_error("Map index must be INT, FLT or STR", it->line, it->column);
+                    goto cleanup;
                 }
 
                 bool last_key_in_node = (i + 1 == indices->count);
@@ -1576,25 +1590,25 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
                     Value* slot = value_map_get_ptr(cur, key, true);
                     value_free(key);
                     if (!slot) {
-                        free(nodes);
-                        return make_error("Internal error assigning to map", stmt_line, stmt_col);
+                        out = make_error("Internal error assigning to map", stmt_line, stmt_col);
+                        goto cleanup;
                     }
                     if (slot->type != VAL_NULL && value_type_to_decl(slot->type) != value_type_to_decl(rhs.type)) {
-                        free(nodes);
-                        return make_error("Map entry type mismatch", stmt_line, stmt_col);
+                        out = make_error("Map entry type mismatch", stmt_line, stmt_col);
+                        goto cleanup;
                     }
                     value_free(*slot);
                     *slot = value_copy(rhs);
-                    free(nodes);
-                    return make_ok(value_null());
+                    out = make_ok(value_null());
+                    goto cleanup;
                 }
 
                 // Descend into slot.
                 Value* slot = value_map_get_ptr(cur, key, true);
                 value_free(key);
                 if (!slot) {
-                    free(nodes);
-                    return make_error("Internal error indexing map", stmt_line, stmt_col);
+                    out = make_error("Internal error indexing map", stmt_line, stmt_col);
+                    goto cleanup;
                 }
 
                 if (slot->type == VAL_NULL) {
@@ -1608,21 +1622,26 @@ ExecResult assign_index_chain(Interpreter* interp, Env* env, Expr* idx_expr, Val
         }
 
         // Unsupported type for indexed assignment
-        free(nodes);
-        return make_error("Indexing assignment is supported only on tensors and maps", node->line, node->column);
+        out = make_error("Indexing assignment is supported only on tensors and maps", node->line, node->column);
+        goto cleanup;
     }
 
     // If we get here, the chain ended after resolving to a tensor element (e.g. a<1> = rhs)
     if (cur->type != VAL_NULL && value_type_to_decl(cur->type) != value_type_to_decl(rhs.type)) {
-        free(nodes);
-        return make_error("Element type mismatch", stmt_line, stmt_col);
+        out = make_error("Element type mismatch", stmt_line, stmt_col);
+        goto cleanup;
     }
     mtx_lock(&g_tns_lock);
     value_free(*cur);
     *cur = value_copy(rhs);
     mtx_unlock(&g_tns_lock);
+
+    out = make_ok(value_null());
+
+cleanup:
     free(nodes);
-    return make_ok(value_null());
+    value_free(base_val);
+    return out;
 }
 
 static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap* labels) {