Add typed native handle drops - #27
Conversation
|
@coderabbitai[bot] please review |
📝 WalkthroughWalkthroughThe change adds typed owned-opaque values for interfaces and HTTP requests, emits backend-specific clone/drop operations, deepens struct-key map ownership handling, adds native ownership fixtures and sanitizer coverage, and updates memory-safety documentation. ChangesNative ownership model
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant NativeIR
participant NativeCodegen
participant NativeBridge
participant NativeRuntime
participant OwnershipFixture
NativeIR->>NativeCodegen: emit typed clone/drop operations
NativeCodegen->>NativeBridge: invoke interface or HTTP request helpers
NativeCodegen->>NativeRuntime: invoke typed struct-map operations
NativeBridge->>NativeBridge: release owned payloads
NativeRuntime->>NativeRuntime: clone or drop map keys and values
OwnershipFixture->>NativeIR: compile ownership scenarios
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/llvm_codegen.rs (1)
945-991: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inst::NullHeaphas noOwnedOpaquearm.Lines 94 and 358 add
OwnedOpaqueto the pointer-ABI groups, butNullHeapstill falls into the_ =>error arm, so any IR that null-initializes an owned-opaque field (inactive enum slot, zero-value struct field) would fail codegen with "NullHeap only applies to heap types". Cheap to include it in the null-pointer group if that path is reachable.🛡️ Proposed change
| Type::ShareInt | Type::FloatSlice | Type::ByteSlice + | Type::OwnedOpaque(_) | Type::BoolSlice => { context.ptr_type(Default::default()).const_null().into() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/llvm_codegen.rs` around lines 945 - 991, Update the Inst::NullHeap type match to include Type::OwnedOpaque in the existing null-pointer group alongside the other pointer-ABI heap types. Preserve the current null-pointer value construction and unsupported-type error behavior for all other types.
🧹 Nitpick comments (3)
src/native_ir.rs (1)
30306-30337: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the drop-kind codes shared with the runtime.
The
1..=8codes here form an untyped contract withruntime/native_runtime.c. TheOwnedOpaquearms are spelled out perOpaqueKind, so a new opaque kind is a compile error — good — but any new heap-carryingMapValKindsilently falls into_ => (0, None), i.e. "no drop", which leaks rather than failing loudly.Consider a small
#[repr(i64)] enum ValueDropKind(orconstset) mirrored by named constants on the C side so the two sides can't drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/native_ir.rs` around lines 30306 - 30337, Replace the untyped drop-kind literals in struct_map_value_meta with a shared, explicitly named ValueDropKind representation covering codes 1 through 8, and mirror those named values in runtime/native_runtime.c. Ensure every heap-carrying MapValKind is matched explicitly so unsupported additions fail at compile time instead of falling through to the no-drop code 0.runtime/native_runtime.c (2)
1616-1623: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
mako_native_struct_drop_typedis a pure alias formako_native_struct_key_free.If the only purpose is an exported name for the compiler-emitted drop, consider documenting that explicitly (or exporting
mako_native_struct_key_freewith a comment) so future readers don't add divergent behavior to one of the two.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/native_runtime.c` around lines 1616 - 1623, Document that mako_native_struct_drop_typed intentionally aliases mako_native_struct_key_free for the compiler-emitted drop operation, and clarify that it must remain behaviorally identical rather than gaining separate logic.
1637-1655: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
mako_native_struct_slice_clone_ptrandmako_native_ptr_slice_clone_typedare the same function modulo the element callback.Same for the two drop variants at Lines 1657-1672 and 1785-1801.
struct_slice_clone_ptris justptr_slice_clone_typedwithvalue_kind == 2; the typed variants could subsume both.♻️ Sketch
-MakoNativeStructSlice *mako_native_struct_slice_clone_ptr( - const MakoNativeStructSlice *in, int64_t nfields, int64_t str_mask, - int64_t nest_mask, int64_t nest_nf_pack, int64_t nest_sm_pack -) { ... } +/* struct element slices are just typed slices with value_kind == 2 */ +#define mako_native_struct_slice_clone_ptr(in, nf, sm, nm, nnf, nsm) \ + mako_native_ptr_slice_clone_typed((in), 2, (nf), (sm), (nm), (nnf), (nsm))Also applies to: 1763-1783
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/native_runtime.c` around lines 1637 - 1655, Consolidate the duplicated cloning and dropping logic by making mako_native_struct_slice_clone_ptr delegate to mako_native_ptr_slice_clone_typed with value_kind 2, and apply the same delegation to the corresponding struct and typed drop functions. Preserve all existing arguments, ownership behavior, allocation handling, and element-cloning semantics while removing the duplicate implementations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 118-126: Update the earlier native-backend paragraph in the
changelog to scope its sanitizer limitation accurately, removing or qualifying
the claim that sanitizers never run and that --sanitize is universally rejected.
Keep the newer statement about native fixtures passing under LeakSanitizer and
AddressSanitizer consistent with the revised wording.
In `@scripts/memory-safety-gate.sh`:
- Around line 124-127: Replace the fixed /tmp/mako-ms-native-asan.out path in
the native ASan test block with a securely created mktemp file, reuse that path
for both command output redirection and the tail command, and clean it up after
use.
In `@src/native_codegen.rs`:
- Line 1730: Update the new declarations in the surrounding match arms,
including the lines calling module.declare_func_in_func, to pass the function
builder in the same non-borrowed form used by neighboring arms. Remove the
unnecessary borrow while preserving the existing function declaration behavior
and avoiding Clippy’s needless_borrow warning.
---
Outside diff comments:
In `@src/llvm_codegen.rs`:
- Around line 945-991: Update the Inst::NullHeap type match to include
Type::OwnedOpaque in the existing null-pointer group alongside the other
pointer-ABI heap types. Preserve the current null-pointer value construction and
unsupported-type error behavior for all other types.
---
Nitpick comments:
In `@runtime/native_runtime.c`:
- Around line 1616-1623: Document that mako_native_struct_drop_typed
intentionally aliases mako_native_struct_key_free for the compiler-emitted drop
operation, and clarify that it must remain behaviorally identical rather than
gaining separate logic.
- Around line 1637-1655: Consolidate the duplicated cloning and dropping logic
by making mako_native_struct_slice_clone_ptr delegate to
mako_native_ptr_slice_clone_typed with value_kind 2, and apply the same
delegation to the corresponding struct and typed drop functions. Preserve all
existing arguments, ownership behavior, allocation handling, and element-cloning
semantics while removing the duplicate implementations.
In `@src/native_ir.rs`:
- Around line 30306-30337: Replace the untyped drop-kind literals in
struct_map_value_meta with a shared, explicitly named ValueDropKind
representation covering codes 1 through 8, and mirror those named values in
runtime/native_runtime.c. Ensure every heap-carrying MapValKind is matched
explicitly so unsupported additions fail at compile time instead of falling
through to the no-drop code 0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68a90f0d-02f1-4d2f-add0-f373913c078f
📒 Files selected for processing (10)
CHANGELOG.mddocs/MEMORY_SAFETY.mddocs/ROADMAP.mdexamples/native/owned_handle_drop/native_owned_handle_drop_test.mkoruntime/native_bridge.cruntime/native_runtime.cscripts/memory-safety-gate.shsrc/llvm_codegen.rssrc/native_codegen.rssrc/native_ir.rs
| if ! "$mako_bin" test "$repo_dir/$f" --backend native --sanitize address \ | ||
| >/tmp/mako-ms-native-asan.out 2>&1; then | ||
| echo "memory-safety-gate: native ASan FAILED on $f" >&2 | ||
| tail -20 /tmp/mako-ms-native-asan.out >&2 || true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use mktemp instead of a fixed /tmp path.
A predictable path is symlink/TOCTOU-prone on shared hosts; a pre-created symlink could redirect the log write.
🛡️ Proposed fix
+ ms_asan_log="$(mktemp)"
for f in examples/native/owned_handle_drop/native_owned_handle_drop_test.mko \
examples/testing/map_struct_key_test.mko; do
if ! "$mako_bin" test "$repo_dir/$f" --backend native --sanitize address \
- >/tmp/mako-ms-native-asan.out 2>&1; then
+ >"$ms_asan_log" 2>&1; then
echo "memory-safety-gate: native ASan FAILED on $f" >&2
- tail -20 /tmp/mako-ms-native-asan.out >&2 || true
+ tail -20 "$ms_asan_log" >&2 || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! "$mako_bin" test "$repo_dir/$f" --backend native --sanitize address \ | |
| >/tmp/mako-ms-native-asan.out 2>&1; then | |
| echo "memory-safety-gate: native ASan FAILED on $f" >&2 | |
| tail -20 /tmp/mako-ms-native-asan.out >&2 || true | |
| ms_asan_log="$(mktemp)" | |
| for f in examples/native/owned_handle_drop/native_owned_handle_drop_test.mko \ | |
| examples/testing/map_struct_key_test.mko; do | |
| if ! "$mako_bin" test "$repo_dir/$f" --backend native --sanitize address \ | |
| >"$ms_asan_log" 2>&1; then | |
| echo "memory-safety-gate: native ASan FAILED on $f" >&2 | |
| tail -20 "$ms_asan_log" >&2 || true |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 124-124: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/mako-ms-native-asan.out
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 126-126: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/mako-ms-native-asan.out
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/memory-safety-gate.sh` around lines 124 - 127, Replace the fixed
/tmp/mako-ms-native-asan.out path in the native ASan test block with a securely
created mktemp file, reuse that path for both command output redirection and the
tail command, and clean it up after use.
Source: Linters/SAST tools
a9b189b to
5c29494
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
runtime/native_runtime.c (2)
2139-2143: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKind 7 (interface) falls through to
default: return 0— make the intent explicit.Every other kind in the 1-8 range is enumerated, so the omission reads as an oversight rather than a deliberate "interfaces compare by identity only" choice (which the
a == bfast path at Line 2089 already provides). An explicitcase 7:with a one-line comment would settle it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/native_runtime.c` around lines 2139 - 2143, Update the kind-dispatch switch containing cases 1–8 to add an explicit case 7 before the default branch, preserving its existing return-0 behavior and documenting that interfaces compare by identity only; leave the existing a == b fast path and other kind handlers unchanged.
1632-1636: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a shared header for these cross-TU prototypes.
These hand-written declarations must stay byte-identical with
runtime/native_bridge.c(mako_native_iface_clone,mako_native_iface_drop,mako_native_http_request_{clone,eq,drop}_ptr). A divergence compiles cleanly and breaks only at link/run time. A shared header would let the compiler enforce it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/native_runtime.c` around lines 1632 - 1636, The cross-translation-unit prototypes around mako_native_iface_clone, mako_native_iface_drop, and mako_native_http_request_{clone,eq,drop}_ptr are duplicated and can diverge. Move these declarations into a shared header, include it from both runtime/native_runtime.c and runtime/native_bridge.c, and remove the hand-written duplicates while preserving the existing signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@runtime/native_runtime.c`:
- Around line 2883-2896: Fix ownership handling in the maps_values_struct_key
result: do not mark the returned slice as owned while val_kind 3–6 elements
remain shared views. Either clone those kinds before storing them, or set
out->owned consistently so mako_native_ptr_slice_drop_typed skips borrowed
elements; preserve safe cleanup for all other value kinds.
- Around line 2775-2777: In runtime/native_runtime.c lines 2775-2777, update
mako_native_maps_keys_struct_ptr to check !m || !m->state before calling
mako_native_map_ii_len(m), returning an empty int slice through the existing
allocation path. Apply the same ordering fix in
mako_native_maps_values_struct_key at lines 2876-2884, preserving the
empty-slice early return and preventing dereference of an invalid map.
- Around line 2013-2039: Add an early return for dst == src at the start of the
struct-map copy/purge function, before the destination cleanup loop. Preserve
the existing behavior for distinct source and destination maps, including null
handling and the subsequent copy flow.
---
Nitpick comments:
In `@runtime/native_runtime.c`:
- Around line 2139-2143: Update the kind-dispatch switch containing cases 1–8 to
add an explicit case 7 before the default branch, preserving its existing
return-0 behavior and documenting that interfaces compare by identity only;
leave the existing a == b fast path and other kind handlers unchanged.
- Around line 1632-1636: The cross-translation-unit prototypes around
mako_native_iface_clone, mako_native_iface_drop, and
mako_native_http_request_{clone,eq,drop}_ptr are duplicated and can diverge.
Move these declarations into a shared header, include it from both
runtime/native_runtime.c and runtime/native_bridge.c, and remove the
hand-written duplicates while preserving the existing signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f04c6b7-9c66-4bfb-b394-2944fbc66249
📒 Files selected for processing (10)
CHANGELOG.mddocs/MEMORY_SAFETY.mddocs/ROADMAP.mdexamples/native/owned_handle_drop/native_owned_handle_drop_test.mkoruntime/native_bridge.cruntime/native_runtime.cscripts/memory-safety-gate.shsrc/llvm_codegen.rssrc/native_codegen.rssrc/native_ir.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- CHANGELOG.md
- docs/ROADMAP.md
- docs/MEMORY_SAFETY.md
- src/llvm_codegen.rs
- examples/native/owned_handle_drop/native_owned_handle_drop_test.mko
- runtime/native_bridge.c
- src/native_ir.rs
| int64_t nest_mask, int64_t nest_nf_pack, int64_t nest_sm_pack, | ||
| int64_t val_kind, int64_t val_nfields, int64_t val_str_mask, | ||
| int64_t val_nest_mask, int64_t val_nest_nf_pack, int64_t val_nest_sm_pack | ||
| ) { | ||
| if (dst && dst->state) { | ||
| uint8_t *st = mako_native_map_ii_state(dst); | ||
| int64_t *dk = mako_native_map_ii_keys(dst); | ||
| int64_t *dv = mako_native_map_ii_vals(dst); | ||
| for (size_t i = 0; i < dst->cap; ++i) { | ||
| if (st[i] == MAKO_NMAP_FULL) { | ||
| mako_native_struct_key_free( | ||
| (void *)(intptr_t)dk[i], nfields, str_mask, | ||
| nest_mask, nest_nf_pack, nest_sm_pack | ||
| ); | ||
| mako_native_struct_map_value_drop( | ||
| dv[i], val_kind, val_nfields, val_str_mask, | ||
| val_nest_mask, val_nest_nf_pack, val_nest_sm_pack | ||
| ); | ||
| dk[i] = 0; | ||
| dv[i] = 0; | ||
| } | ||
| st[i] = MAKO_NMAP_EMPTY; | ||
| } | ||
| if (dst->lenp) *dst->lenp = 0; | ||
| dst->tombs = 0; | ||
| } | ||
| if (!src || !dst) return; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Add a dst == src short-circuit.
The purge loop frees every destination key/value and marks all buckets EMPTY before the copy loop runs. If dst == src, the copy loop then sees only EMPTY buckets and the map is silently wiped instead of left unchanged. Cheap to guard.
🛡️ Proposed fix
+ if (dst == src) return;
if (dst && dst->state) {
uint8_t *st = mako_native_map_ii_state(dst);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int64_t nest_mask, int64_t nest_nf_pack, int64_t nest_sm_pack, | |
| int64_t val_kind, int64_t val_nfields, int64_t val_str_mask, | |
| int64_t val_nest_mask, int64_t val_nest_nf_pack, int64_t val_nest_sm_pack | |
| ) { | |
| if (dst && dst->state) { | |
| uint8_t *st = mako_native_map_ii_state(dst); | |
| int64_t *dk = mako_native_map_ii_keys(dst); | |
| int64_t *dv = mako_native_map_ii_vals(dst); | |
| for (size_t i = 0; i < dst->cap; ++i) { | |
| if (st[i] == MAKO_NMAP_FULL) { | |
| mako_native_struct_key_free( | |
| (void *)(intptr_t)dk[i], nfields, str_mask, | |
| nest_mask, nest_nf_pack, nest_sm_pack | |
| ); | |
| mako_native_struct_map_value_drop( | |
| dv[i], val_kind, val_nfields, val_str_mask, | |
| val_nest_mask, val_nest_nf_pack, val_nest_sm_pack | |
| ); | |
| dk[i] = 0; | |
| dv[i] = 0; | |
| } | |
| st[i] = MAKO_NMAP_EMPTY; | |
| } | |
| if (dst->lenp) *dst->lenp = 0; | |
| dst->tombs = 0; | |
| } | |
| if (!src || !dst) return; | |
| if (dst == src) return; | |
| if (dst && dst->state) { | |
| uint8_t *st = mako_native_map_ii_state(dst); | |
| int64_t *dk = mako_native_map_ii_keys(dst); | |
| int64_t *dv = mako_native_map_ii_vals(dst); | |
| for (size_t i = 0; i < dst->cap; ++i) { | |
| if (st[i] == MAKO_NMAP_FULL) { | |
| mako_native_struct_key_free( | |
| (void *)(intptr_t)dk[i], nfields, str_mask, | |
| nest_mask, nest_nf_pack, nest_sm_pack | |
| ); | |
| mako_native_struct_map_value_drop( | |
| dv[i], val_kind, val_nfields, val_str_mask, | |
| val_nest_mask, val_nest_nf_pack, val_nest_sm_pack | |
| ); | |
| dk[i] = 0; | |
| dv[i] = 0; | |
| } | |
| st[i] = MAKO_NMAP_EMPTY; | |
| } | |
| if (dst->lenp) *dst->lenp = 0; | |
| dst->tombs = 0; | |
| } | |
| if (!src || !dst) return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/native_runtime.c` around lines 2013 - 2039, Add an early return for
dst == src at the start of the struct-map copy/purge function, before the
destination cleanup loop. Preserve the existing behavior for distinct source and
destination maps, including null handling and the subsequent copy flow.
| int64_t n = (int64_t)mako_native_map_ii_len(m); | ||
| MakoNativeIntSlice *out = mako_native_int_slice_make_ptr((size_t)n, (size_t)n); | ||
| if (!m || !m->state || !n) return out; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
NULL guard runs after m is already dereferenced in both new map-extraction helpers. Each function computes mako_native_map_ii_len(m) first and only then checks !m, so the guard is either dead code or arrives one dereference too late.
runtime/native_runtime.c#L2775-L2777: inmako_native_maps_keys_struct_ptr, hoist the!m || !m->statecheck above themako_native_map_ii_len(m)call and return an empty int slice early.runtime/native_runtime.c#L2876-L2884: inmako_native_maps_values_struct_key, hoist the same!m || !m->statecheck above themako_native_map_ii_len(m)call at Line 2876, keeping the empty-slice allocation for the early return.
📍 Affects 1 file
runtime/native_runtime.c#L2775-L2777(this comment)runtime/native_runtime.c#L2876-L2884
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/native_runtime.c` around lines 2775 - 2777, In
runtime/native_runtime.c lines 2775-2777, update
mako_native_maps_keys_struct_ptr to check !m || !m->state before calling
mako_native_map_ii_len(m), returning an empty int slice through the existing
allocation path. Apply the same ordering fix in
mako_native_maps_values_struct_key at lines 2876-2884, preserving the
empty-slice early return and preventing dereference of an invalid map.
| out->owned = 1; | ||
| if (!m || !m->state || !n) return out; | ||
| size_t j = 0; | ||
| for (size_t i = 0; i < m->cap; ++i) { | ||
| if (m->state[i] == MAKO_NMAP_FULL) { | ||
| int64_t value = m->vals[i]; | ||
| /* Slice values remain shared views, matching ordinary maps_values. */ | ||
| if (val_kind < 3 || val_kind > 6) { | ||
| value = mako_native_struct_map_value_clone( | ||
| value, val_kind, val_nfields, val_str_mask, | ||
| val_nest_mask, val_nest_nf_pack, val_nest_sm_pack | ||
| ); | ||
| } | ||
| out->data[j++] = (void *)(intptr_t)value; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
out->owned = 1 contradicts leaving kinds 3-6 as shared views.
Elements for val_kind 3-6 are borrowed from the map, but the slice is marked owned. Dropping it through mako_native_ptr_slice_drop_typed (Line 1786) will deep-drop those borrowed slices, double-freeing map-owned storage. Either clone kinds 3-6 as well, or set owned = 0 for those kinds so the drop path skips element release.
#!/bin/bash
# Confirm how the returned slice from maps_values_struct_key is released.
rg -nP -C6 'maps_values_struct_key|ptr_slice_drop_typed'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@runtime/native_runtime.c` around lines 2883 - 2896, Fix ownership handling in
the maps_values_struct_key result: do not mark the returned slice as owned while
val_kind 3–6 elements remain shared views. Either clone those kinds before
storing them, or set out->owned consistently so mako_native_ptr_slice_drop_typed
skips borrowed elements; preserve safe cleanup for all other value kinds.
2 participants
Adds typed ownership for runtime-managed native handles while keeping foreign handles on their explicit close paths.
Interface boxes are reference-counted and use compiler-generated recursive destructors. HTTP requests are deep-cloned, struct-key maps release owned keys and values across their lifecycle, and unsupported metadata layouts now fail at compile time.
Verified with:
cargo test --bin mako— 136 passedSummary by CodeRabbit