NO-REVIEW try to enable finalizers on top of CallDescrWorkerInternal change by radekdoulik · Pull Request #124789 · dotnet/runtime

radekdoulik · 2026-02-24T11:22:12Z

No description provided.

To call it before we potentially put object references on stack, because DoPrestub can trigger GC.

dotnet-policy-service · 2026-02-24T11:23:03Z

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR attempts to enable Browser/WASM finalization callbacks and refactors WASM portable-entrypoint call paths to prestub/initialize interpreter code outside of CallDescrWorkerInternal (to avoid GC-safety issues during argument setup).

Changes:

Removes the in-worker prestub path from CallDescrWorkerInternal and replaces it with an assertion that interpreter code is already available.
Adds a new MethodDesc::GetInterpreterCodeWithPrestub() helper and calls it from several invocation paths to force interpreter-code initialization.
Enables Browser/WASM finalization callback execution by calling ManagedThreadBase::KickOff(FinalizerThreadWorkerIteration, ...).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/coreclr/vm/wasm/calldescrworkerwasm.cpp	Removes prestub-in-worker and assumes interpreter code is already initialized.
src/coreclr/vm/reflectioninvocation.cpp	Forces prestub/interpreter code initialization before reflection invoke call frame setup.
src/coreclr/vm/method.hpp	Introduces `GetInterpreterCodeWithPrestub()` helper.
src/coreclr/vm/finalizerthread.cpp	Enables WASM Browser finalization callback to run finalizer iteration.
src/coreclr/vm/callhelpers.cpp	Attempts to prestub/initialize interpreter code before certain VM->managed call helpers.

Copilot · 2026-02-24T11:27:50Z

src/coreclr/vm/callhelpers.cpp

+#ifdef FEATURE_PORTABLE_ENTRYPOINTS
+    MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(m_pCallTarget);
+    (void)pMethod->GetInterpreterCodeWithPrestub();
+#endif // FEATURE_PORTABLE_ENTRYPOINTS
+


MethodDescCallSite::CallTargetWorker explicitly warns that it must not trigger a GC because the incoming argument array is not GC-protected. GetInterpreterCodeWithPrestub() may call DoPrestub and trigger GC, violating that requirement and potentially corrupting GC refs in pArguments. The prestub/interpreter-code initialization needs to be moved to a point where arguments are still protected (e.g., before constructing/filling the ARG_SLOT array) or otherwise guaranteed non-GC.

Suggested change

#ifdef FEATURE_PORTABLE_ENTRYPOINTS

MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(m_pCallTarget);

(void)pMethod->GetInterpreterCodeWithPrestub();

#endif // FEATURE_PORTABLE_ENTRYPOINTS

Copilot · 2026-02-24T11:27:50Z

src/coreclr/vm/wasm/calldescrworkerwasm.cpp

    MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(pCallDescrData->pTarget);
    InterpByteCodeStart* targetIp = pMethod->GetInterpreterCode();
-    if (targetIp == NULL)
-    {
-        GCX_PREEMP();
-        (void)pMethod->DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);
-        targetIp = pMethod->GetInterpreterCode();
-    }
+    _ASSERTE(targetIp != nullptr);


CallDescrWorkerInternal now only _ASSERTEs that GetInterpreterCode() is non-null. In release builds, a null targetIp would be passed to ExecuteInterpretedMethodWithArgs, likely causing a crash. If the intent is to require prestubbing earlier, this should be enforced in all builds (fail fast) and/or targetIp should be obtained via a guaranteed-initializing path, with the initialization performed before any unprotected argument references are placed in the call frame.

Copilot · 2026-02-24T11:27:51Z

src/coreclr/vm/method.hpp

+        GCX_PREEMP();
+        (void)DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);
+        return GetInterpreterCode();


GetInterpreterCodeWithPrestub() can return NULL even after DoPrestub() (e.g., if interpreter code is poisoned/unavailable), but current call sites ignore the return value and later assume interpreter code exists. Consider asserting/checking that interpreter code is non-null after prestub (ideally in all builds) so failures are caught deterministically rather than surfacing later as null derefs.

Suggested change

GCX_PREEMP();

(void)DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);

return GetInterpreterCode();

// If interpreter code has been explicitly poisoned, it is a misuse to call

// GetInterpreterCodeWithPrestub; there is no valid interpreter code to obtain.

if (IsInterpreterCodePoisoned())

{

_ASSERTE(!"GetInterpreterCodeWithPrestub called on method with poisoned interpreter code");

return NULL;

}

GCX_PREEMP();

(void)DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);

targetIp = GetInterpreterCode();

// After prestub, interpreter code should be available unless it was poisoned.

_ASSERTE(targetIp != NULL || IsInterpreterCodePoisoned());

return targetIp;

Copilot · 2026-02-24T11:27:51Z

src/coreclr/vm/finalizerthread.cpp

@@ -59,7 +59,7 @@ extern "C" void SystemJS_ExecuteFinalizationCallback()
    {
        GCX_COOP();
        // TODO-WASM https://github.com/dotnet/runtime/issues/123712


The TODO-WASM comment referencing issue 123712 is now misleading because the finalization kickoff is enabled immediately below it. Either remove the TODO or update it to describe the remaining limitation/work item so future readers don’t assume finalization is still disabled here.

Suggested change

// TODO-WASM https://github.com/dotnet/runtime/issues/123712

Copilot · 2026-02-24T11:27:51Z

src/coreclr/vm/callhelpers.cpp

+#ifdef FEATURE_PORTABLE_ENTRYPOINTS
+    MethodDesc* pMethod = PortableEntryPoint::GetMethodDesc(pTargetAddress);
+    (void)pMethod->GetInterpreterCodeWithPrestub();
+#endif // FEATURE_PORTABLE_ENTRYPOINTS


DispatchCallSimple is called with an argument buffer (pSrc / __pArgs) that often contains raw object pointers produced by OBJECTREF_TO_ARGHOLDER. Adding GetInterpreterCodeWithPrestub() here can trigger a GC via DoPrestub, which can invalidate those unprotected object pointers before the call is made. The prestub/interpreter-code initialization needs to happen earlier, before arguments are materialized as unmanaged pointers (or in a path that guarantees GC cannot occur).

radekdoulik added 2 commits February 24, 2026 12:10

Move DoPrestub call out of CallDescrWorkerInternal

772f2b0

To call it before we potentially put object references on stack, because DoPrestub can trigger GC.

Try to enable finalizer call

a60b435

radekdoulik added this to the Future milestone Feb 24, 2026

Copilot AI review requested due to automatic review settings February 24, 2026 11:22

radekdoulik added NO-REVIEW Experimental/testing PR, do NOT review it area-VM-coreclr labels Feb 24, 2026

dotnet-policy-service bot assigned radekdoulik Feb 24, 2026

Copilot started reviewing on behalf of radekdoulik February 24, 2026 11:23 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

radekdoulik closed this Feb 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NO-REVIEW try to enable finalizers on top of CallDescrWorkerInternal change#124789

NO-REVIEW try to enable finalizers on top of CallDescrWorkerInternal change#124789
radekdoulik wants to merge 2 commits intodotnet:mainfrom
radekdoulik:clr-wasm-try-to-enable-finalizers

radekdoulik commented Feb 24, 2026

Uh oh!

dotnet-policy-service bot commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Copilot AI Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        GCX_PREEMP();
-        (void)DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);
-        return GetInterpreterCode();
+        // If interpreter code has been explicitly poisoned, it is a misuse to call
+        // GetInterpreterCodeWithPrestub; there is no valid interpreter code to obtain.
+        if (IsInterpreterCodePoisoned())
+        {
+            _ASSERTE(!"GetInterpreterCodeWithPrestub called on method with poisoned interpreter code");
+            return NULL;
+        }
+        GCX_PREEMP();
+        (void)DoPrestub(NULL /* MethodTable */, CallerGCMode::Coop);
+        targetIp = GetInterpreterCode();
+        // After prestub, interpreter code should be available unless it was poisoned.
+        _ASSERTE(targetIp != NULL || IsInterpreterCodePoisoned());
+        return targetIp;

Conversation

radekdoulik commented Feb 24, 2026

Uh oh!

dotnet-policy-service bot commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants