fix: Re-apply fix for CFunctionInfo/CTypeInfo leak in OpCtx

[svix-aaron1011]

No description provided.

[mmastrac]

I think we may need to revisit the code in rusty_v8 -- creating the CTypeInfo and CFunctionInfo should probably yield UniqueRefs rather than NonNull pointers.

I'm not 100% sure, but I think the temporary reference is actually unsound here.

[svix-aaron1011]

There aren't any temporary references being created here - as_ptr() gives us a raw pointer, which gets passed directly to std::ptr::drop_in_place without creating a reference.

[mmastrac]

@svix-aaron1011 I think you might need to revisit v8__CFunctionInfo__DELETE and have that take a pointer. I know that we have a few other APIs that pass refs across extern "C", but I don't believe it's sound as the mutable reference cannot be invalidated by a call.

While we're in there, we should fix the other places we do this as well

[svix-aaron1011]

Now that rusty_v8 has been bumped to include denoland/rusty_v8#1471, is there anything else I need to do for this PR?

[mmastrac]

I think we just need to re-run a build of deno + tests w/this patch and then we can re-land

[svix-aaron1011]

I've rebase the pr, and the tests are passing

[bartlomieju]

@svix-aaron1011, sorry the delay. I'll open a PR in deno to check if we're all good and then land the PR.

[bartlomieju]

I tried it in denoland/deno#24142 and I'm getting:

test util::fs::tests::lax_fs_lock_ordered ... ok
test worker::tests::execute_mod_circular ... ok
Hello World
test worker::tests::execute_mod_002_hello ... ok
test worker::tests::execute_mod_resolve_error ... ok
test worker::tests::execute_mod_esm_imports_a ... ok
test lsp::tsc::tests::test_remote_modules ... ok
test lsp::tsc::tests::test_get_diagnostics_lib ... ok
test tools::test::channel::tests::test_interleave ... ok
test tsc::tests::test_exec_basic ... ok
test tsc::tests::test_exec_reexport_dts ... ok


#
# Fatal error in , line 0
# Check failed: index < node->op()->ValueInputCount().
#
#
#
#FailureMessage Object: 0xffea1efeb7e0
==== C stack trace ===============================

    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x665df08) [0xab6e12e9df08]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5cf8374) [0xab6e12538374]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5cf5c48) [0xab6e12535c48]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x6b7cd68) [0xab6e133bcd68]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x6b69b50) [0xab6e133a9b50]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x6b50dac) [0xab6e13390dac]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x6b2026c) [0xab6e1336026c]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x69b84f0) [0xab6e131f84f0]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x69b39c8) [0xab6e131f39c8]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x69b30a8) [0xab6e131f30a8]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5d4f758) [0xab6e1258f758]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5d8355c) [0xab6e125c355c]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5d84e54) [0xab6e125c4e54]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x6663d84) [0xab6e12ea3d84]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5cf9888) [0xab6e12539888]
    /home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775(+0x5cf76bc) [0xab6e125376bc]
    /lib/aarch64-linux-gnu/libc.so.6(+0x7d5c8) [0xffea5d5ed5c8]
    /lib/aarch64-linux-gnu/libc.so.6(+0xe5edc) [0xffea5d655edc]
error: test failed, to rerun pass `-p deno --bin deno`

Caused by:
  process didn't exit successfully: `/home/runner/work/deno/deno/target/debug/deps/deno-1a34928f818fb775` (signal: 5, SIGTRAP: trace/breakpoint trap)

https://github.com/denoland/deno/actions/runs/9423154307/job/25960976323?pr=24142

Calling @devsnek for help with this one.

[devsnek]

looks like a bug in turbofan? I've never seen code free CFunctionInfo before (it's usually statically allocated...) so I'm not totally sure what the right thing to do is, I'd need to take a look at V8 source.

[bartlomieju]

Another try in denoland/deno#24169

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.50%. Comparing base (0c7f83e) to head (40525a2).
Report is 30 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #714      +/-   ##
==========================================
+ Coverage   81.43%   81.50%   +0.06%     
==========================================
  Files          97       97              
  Lines       23877    24027     +150     
==========================================
+ Hits        19445    19583     +138     
- Misses       4432     4444      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.