Is this the same change we've tried to make several times before and it caused crashes in production every time but no one ever debugged it?

Is this the same change we've tried to make several times before and it caused crashes in production every time but no one ever debugged it?

Yes. I've tried it twice at least

First attempt: #714
Reverted: #716
Second attempt: #909
Reverted: #916

I wasn't sure but yes it is. Same as #714, currently we patch in the old version in patches/v8/0011-Revert-TracedReference-deref-API-removal.patch.

Both reverts only say it caused trouble in tests, though.

I thought there was a time it got to prod and caused crashes but maybe I'm misremembering.

Fifth attempt: #4122

Probably the problem is that handle.Get(isolate) creates a new local handle, but this callback is called during garbage collection, which is a bad time for creating new handles. We need some other way to get an the internal pointers, without any side effects.

It's sort of dumb to make handles during a GC, since they are roots, and the number of roots should not change during GC, but the new API requires it. However I think it's enough to just create a HandleScope so that the newly created handles are quickly destroyed again before GC continues. Let's see if that fixes it (or was that also already tried)?

I'm also trying to upstream https://chromium-review.googlesource.com/c/v8/v8/+/6652167 to make this footgun less subtle.

I checked the linked attempts here, and none of them added the HandleScope.