Fix memory leak when a behavior parameter has a different capability than the trait it implements

When a behavior is called through a trait and the concrete actor's parameter
has a different trace-significant capability (e.g., trait has iso, concrete
has val), the sender traces with one trace kind and the receiver traces with
another. This ORCA GC mismatch causes field reference counts to never reach
zero, leaking objects reachable from the parameter.

The fix includes trace-kind characters in mangled names for behaviors and
constructors, so methods whose parameters differ in trace-significant ways
get distinct vtable indices. When a forwarding method is created due to a
cap mismatch, genfun_forward now adds a dispatch case that traces with the
forwarding method's params (the trait's capabilities) but calls the concrete
method's handler. A two-pass ordering in genfun_method_bodies ensures
concrete handlers are generated before forwarding dispatch cases that
reference them.

The leak is not currently active because make_might_reference_actor forces
full tracing of immutable objects. Without this fix, re-enabling that
optimization would expose the leak.

Design: #4943
Closes #4102

Author: SeanTAllen

.release-notes/fix-iso-to-val-trait-memory-leak.md

@@ -0,0 +1,17 @@
+## Fix memory leak
+
+When a behavior was called through a trait reference and the concrete actor's parameter had a different trace-significant capability (e.g., the trait declared `iso` but the actor declared `val`), the ORCA garbage collector's reference counting was broken. The sender traced the parameter with one trace kind and the receiver traced with another, causing field reference counts to never reach zero. Objects reachable from the parameter were leaked.
+
+```pony
+trait tag Receiver
+  be receive(b: SomeClass iso)
+
+actor MyActor is Receiver
+  be receive(b: SomeClass val) =>
+    // b's fields were leaked when called through a Receiver reference
+    None
+```
+
+The leak is not currently active because `make_might_reference_actor` — an optimization that was disabled as a safety net — masks it by forcing full tracing of all immutable objects. Without this fix, the leak would have become active as we started re-enabling that optimization.
+
+The sender and receiver now use consistent tracing for each parameter, regardless of capability differences between the trait and concrete method.

src/libponyc/codegen/genfun.c

@@ -818,6 +818,43 @@ static bool genfun_forward(compile_t* c, reach_type_t* t,
   genfun_build_ret(c, ret);
   codegen_finishfun(c);
   ponyint_pool_free_size(buf_size, args);
+
+  // For forwarding behaviors/constructors on actors, add a dispatch case that
+  // traces with the forwarding method's params (the trait's capabilities) but
+  // calls the concrete method's handler.
+  //
+  // Only do this when all parameter types match between the forwarding and
+  // concrete methods — i.e. the forwarding was created solely because of a
+  // trace-kind (cap) difference. When the types themselves differ (e.g. a
+  // tuple vs Any), the LLVM types are incompatible and the existing forwarding
+  // sender path handles dispatch correctly without a separate dispatch case.
+  if(t->underlying == TK_ACTOR)
+  {
+    token_id fun_id = ast_id(m->fun->ast);
+
+    if((fun_id == TK_BE) || (fun_id == TK_NEW))
+    {
+      bool types_match = (m->param_count == m2->param_count);
+
+      for(size_t i = 0; types_match && (i < m->param_count); i++)
+      {
+        if(m->params[i].type != m2->params[i].type)
+          types_match = false;
+      }
+
+      if(types_match)
+      {
+        pony_assert(c_m2->func_handler != NULL);
+        add_dispatch_case(c, t, m->params, m->vtable_index,
+          c_m2->func_handler, c_m2->func_type, c_m->msg_type);
+
+#if defined(USE_RUNTIME_TRACING)
+        add_get_behavior_name_case(c, t, m->name, m->vtable_index);
+#endif
+      }
+    }
+  }
+
   return true;
 }
 
@@ -890,6 +927,7 @@ void genfun_param_attrs(compile_t* c, reach_type_t* t, reach_method_t* m,
   LLVMValueRef fun)
 {
   LLVM_DECLARE_ATTRIBUTEREF(noalias_attr, noalias, 0);
+  LLVM_DECLARE_ATTRIBUTEREF(readonly_attr, readonly, 0);
 
   LLVMValueRef param = LLVMGetFirstParam(fun);
   reach_type_t* type = t;
@@ -928,9 +966,12 @@ void genfun_param_attrs(compile_t* c, reach_type_t* t, reach_method_t* m,
 
           case TK_TRN:
           case TK_REF:
-          case TK_TAG:
+            break;
+
           case TK_VAL:
+          case TK_TAG:
           case TK_BOX:
+            LLVMAddAttributeAtIndex(fun, i + offset, readonly_attr);
             break;
 
           default:
@@ -1019,19 +1060,45 @@ bool genfun_method_bodies(compile_t* c, reach_type_t* t)
     size_t j = HASHMAP_BEGIN;
     reach_method_t* m;
 
+    // First pass: non-forwarding methods (generates handlers that forwarding
+    // dispatch cases will reference).
     while((m = reach_mangled_next(&n->r_mangled, &j)) != NULL)
     {
-      if(!genfun_method(c, t, n, m))
+      if(!m->forwarding)
       {
-        if(errors_get_count(c->opt->check.errors) == 0)
+        if(!genfun_method(c, t, n, m))
         {
-          pony_assert(m->fun != NULL);
-          ast_error(c->opt->check.errors, m->fun->ast,
-            "internal failure: code generation failed for method %s",
-            m->full_name);
+          if(errors_get_count(c->opt->check.errors) == 0)
+          {
+            pony_assert(m->fun != NULL);
+            ast_error(c->opt->check.errors, m->fun->ast,
+              "internal failure: code generation failed for method %s",
+              m->full_name);
+          }
+
+          return false;
         }
+      }
+    }
 
-        return false;
+    // Second pass: forwarding methods (may need handlers from first pass).
+    j = HASHMAP_BEGIN;
+    while((m = reach_mangled_next(&n->r_mangled, &j)) != NULL)
+    {
+      if(m->forwarding)
+      {
+        if(!genfun_method(c, t, n, m))
+        {
+          if(errors_get_count(c->opt->check.errors) == 0)
+          {
+            pony_assert(m->fun != NULL);
+            ast_error(c->opt->check.errors, m->fun->ast,
+              "internal failure: code generation failed for method %s",
+              m->full_name);
+          }
+
+          return false;
+        }
       }
     }
   }

src/libponyc/reach/reach.c

@@ -212,15 +212,74 @@ static void set_method_types(reach_t* r, reach_method_t* m,
   ast_free_unattached(r_result);
 }
 
+static char trace_kind_char(ast_t* type)
+{
+  switch(ast_id(type))
+  {
+    case TK_NOMINAL:
+    {
+      ast_t* def = (ast_t*)ast_data(type);
+
+      switch(ast_id(def))
+      {
+        case TK_ACTOR:
+          return 't';
+
+        case TK_PRIMITIVE:
+          return 'p';
+
+        default:
+          switch(ast_id(cap_fetch(type)))
+          {
+            case TK_VAL: return 'i';
+            case TK_TAG: return 't';
+            default:     return 'm';
+          }
+      }
+    }
+
+    case TK_UNIONTYPE:
+    case TK_ISECTTYPE:
+    case TK_TUPLETYPE:
+      // Compound types all return 'x'. This means trace-kind mismatches
+      // within union/intersection/tuple parameters won't be distinguished.
+      // The simple nominal case covers the reported bug; compound parameter
+      // mismatches are valid but rare and can be addressed separately.
+      return 'x';
+
+    default:
+      return 'x';
+  }
+}
+
 static const char* make_mangled_name(reach_method_t* m)
 {
   // Generate the mangled name.
-  // cap_name[_Arg1_Arg2]_args_result
+  // cap_name[_Arg1_Arg2]_[<trace_kind>]<args>_result
+  //
+  // Trace kind characters are only included for behaviors and constructors,
+  // since those are the only methods where a trace-kind mismatch between
+  // sender and receiver can cause ORCA GC problems. Functions don't have
+  // dispatch cases, and their symbol names may be hard-coded in the runtime
+  // (e.g. Main_runtime_override_defaults).
+  bool include_trace_kind = false;
+
+  if(m->fun != NULL)
+  {
+    token_id fun_id = ast_id(m->fun->ast);
+    include_trace_kind = (fun_id == TK_BE) || (fun_id == TK_NEW);
+  }
+
   printbuf_t* buf = printbuf_new();
   printbuf(buf, "%s_", m->name);
 
   for(size_t i = 0; i < m->param_count; i++)
+  {
+    if(include_trace_kind)
+      printbuf(buf, "%c", trace_kind_char(m->params[i].ast));
+
     printbuf(buf, "%s", m->params[i].type->mangle);
+  }
 
   if(!m->internal)
     printbuf(buf, "%s", m->result->mangle);
@@ -232,7 +291,7 @@ static const char* make_mangled_name(reach_method_t* m)
 static const char* make_full_name(reach_type_t* t, reach_method_t* m)
 {
   // Generate the full mangled name.
-  // pkg_Type[_Arg1_Arg2]_cap_name[_Arg1_Arg2]_args_result
+  // pkg_Type[_Arg1_Arg2]_cap_name[_Arg1_Arg2]_[<trace_kind>]<args>_result
   printbuf_t* buf = printbuf_new();
   printbuf(buf, "%s_%s", t->name, m->mangled_name);
   const char* name = stringtab(buf->m);

test/full-program-tests/codegen-trace-iso-to-tag-through-trait/expected-exit-code.txt

@@ -0,0 +1 @@
+1

test/full-program-tests/codegen-trace-iso-to-tag-through-trait/main.pony

@@ -0,0 +1,41 @@
+use "lib:codegen-trace-additional"
+
+use @raw_cast[B ref](obj: B tag)
+use @gc_local[Pointer[None]](target: Main)
+use @gc_local_snapshot[Pointer[None]](target: Main)
+use @gc_local_snapshot_destroy[None](obj_map: Pointer[None])
+use @objectmap_has_object[Bool](obj_map: Pointer[None], obj: Any tag)
+use @objectmap_has_object_rc[Bool](obj_map: Pointer[None], obj: Any tag, rc: USize)
+use @pony_exitcode[None](code: I32)
+
+class A
+
+class B
+  let a: A = A
+
+trait tag Receiver
+  be receive(b: B iso)
+
+actor Main is Receiver
+  var map_before: Pointer[None] = Pointer[None]
+
+  new create(env: Env) =>
+    // Call through a trait-typed variable to exercise the forwarding path.
+    let r: Receiver = this
+    r.receive(recover B end)
+    map_before = @gc_local_snapshot(this)
+
+  be receive(b: B tag) =>
+    let b' = @raw_cast(b)
+    let map_after = @gc_local(this)
+    // The sender traces b as MUTABLE (iso from the trait), which recurses into
+    // B's fields, incrementing a.rc. The receiver's dispatch case must also
+    // recurse to decrement a.rc. Without the fix, the dispatch traces as
+    // OPAQUE (tag from the concrete method), skipping field recursion entirely,
+    // leaving a.rc stuck at 1.
+    let ok = @objectmap_has_object_rc(map_before, b', USize(1)) and
+      @objectmap_has_object_rc(map_before, b'.a, USize(1)) and
+      @objectmap_has_object_rc(map_after, b', USize(0)) and
+      @objectmap_has_object_rc(map_after, b'.a, USize(0))
+    @gc_local_snapshot_destroy(map_before)
+    @pony_exitcode(I32(if ok then 1 else 0 end))

test/full-program-tests/codegen-trace-iso-to-val-through-trait/expected-exit-code.txt

@@ -0,0 +1 @@
+1

test/full-program-tests/codegen-trace-iso-to-val-through-trait/main.pony

@@ -0,0 +1,41 @@
+use "lib:codegen-trace-additional"
+
+use @raw_cast[B ref](obj: B tag)
+use @gc_local[Pointer[None]](target: Main)
+use @gc_local_snapshot[Pointer[None]](target: Main)
+use @gc_local_snapshot_destroy[None](obj_map: Pointer[None])
+use @objectmap_has_object[Bool](obj_map: Pointer[None], obj: Any tag)
+use @objectmap_has_object_rc[Bool](obj_map: Pointer[None], obj: Any tag, rc: USize)
+use @pony_exitcode[None](code: I32)
+
+class A
+
+class B
+  let a: A = A
+
+trait tag Receiver
+  be receive(b: B iso)
+
+actor Main is Receiver
+  var map_before: Pointer[None] = Pointer[None]
+
+  new create(env: Env) =>
+    // Call through a trait-typed variable to exercise the forwarding path.
+    let r: Receiver = this
+    r.receive(recover B end)
+    map_before = @gc_local_snapshot(this)
+
+  be receive(b: B val) =>
+    let b' = @raw_cast(b)
+    let map_after = @gc_local(this)
+    // The sender traces b as MUTABLE (iso from the trait), which recurses into
+    // B's fields, incrementing a.rc. The receiver's dispatch case must also
+    // recurse to decrement a.rc. Without the fix, the dispatch traces as
+    // IMMUTABLE (val from the concrete method), skipping field recursion when
+    // might_reference_actor is false, leaving a.rc stuck at 1.
+    let ok = @objectmap_has_object_rc(map_before, b', USize(1)) and
+      @objectmap_has_object_rc(map_before, b'.a, USize(1)) and
+      @objectmap_has_object_rc(map_after, b', USize(0)) and
+      @objectmap_has_object_rc(map_after, b'.a, USize(0))
+    @gc_local_snapshot_destroy(map_before)
+    @pony_exitcode(I32(if ok then 1 else 0 end))

test/full-program-tests/codegen-trace-val-to-val-through-trait/expected-exit-code.txt

@@ -0,0 +1 @@
+1

test/full-program-tests/codegen-trace-val-to-val-through-trait/main.pony

@@ -0,0 +1,41 @@
+use "lib:codegen-trace-additional"
+
+use @gc_local[Pointer[None]](target: Main)
+use @gc_local_snapshot[Pointer[None]](target: Main)
+use @gc_local_snapshot_destroy[None](obj_map: Pointer[None])
+use @objectmap_has_object[Bool](obj_map: Pointer[None], obj: Any tag)
+use @objectmap_has_object_rc[Bool](obj_map: Pointer[None], obj: Any tag, rc: USize)
+use @pony_exitcode[None](code: I32)
+
+class A
+
+class B
+  let a: A = A
+
+trait tag Receiver
+  be receive(b: B val)
+
+actor Main is Receiver
+  var map_before: Pointer[None] = Pointer[None]
+
+  new create(env: Env) =>
+    // Call through a trait-typed variable, same as the iso-to-val test.
+    // Both trait and concrete method agree on val, so no forwarding is created.
+    // This verifies the non-forwarding val path isn't broken by the dispatch
+    // and mangling changes.
+    let r: Receiver = this
+    r.receive(recover B end)
+    map_before = @gc_local_snapshot(this)
+
+  be receive(b: B val) =>
+    let map_after = @gc_local(this)
+    // Both sender and receiver trace b as IMMUTABLE (val). Since B has no
+    // actor fields (might_reference_actor is false), immutable tracing does
+    // not recurse into fields. So b gets rc bookkeeping but b.a does not
+    // appear in the object map at all.
+    let ok = @objectmap_has_object_rc(map_before, b, USize(1)) and
+      not @objectmap_has_object(map_before, b.a) and
+      @objectmap_has_object_rc(map_after, b, USize(0)) and
+      not @objectmap_has_object(map_after, b.a)
+    @gc_local_snapshot_destroy(map_before)
+    @pony_exitcode(I32(if ok then 1 else 0 end))