src: integrate ContextifyContext to cppgc

[joyeecheung]

This simplifies the memory management of ContextifyContext,
making all references visible to V8.

The destructors don't need to do anything because when the wrapper is
going away, the context is already going away or otherwise it would've
been holding the wrapper alive, so there's no need to reset the
pointers in the context. Also, any global handles to the context
would've been empty at this point, and the per-Environment context
tracking code is capable of dealing with empty handles from contexts
purged elsewhere.

To this end, the context tracking code also purges empty handles
from the list now, to prevent keeping too many empty handles around.

[codecov]

Codecov Report

Attention: Patch coverage is 69.23077% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.20%. Comparing base (c8df98d) to head (b18f41a).
Report is 158 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_contextify.cc	61.11%	7 Missing ⚠️
src/node_contextify.h	66.66%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #56522   +/-   ##
=======================================
  Coverage   89.20%   89.20%           
=======================================
  Files         662      662           
  Lines      191816   191818    +2     
  Branches    36926    36923    -3     
=======================================
+ Hits       171102   171111    +9     
+ Misses      13559    13555    -4     
+ Partials     7155     7152    -3     

Files with missing lines	Coverage Δ
src/env.cc	85.87% <100.00%> (+0.12%)	⬆️
src/env.h	98.14% <ø> (ø)
src/node_contextify.h	86.66% <66.66%> (+13.33%)	⬆️
src/node_contextify.cc	81.37% <61.11%> (+<0.01%)	⬆️

... and 34 files with indirect coverage changes

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64425/

[joyeecheung]

After tracing the lifetime of ContextifyContexts a bit I realized that ContextifyContext does not even need to do any special cleanups, as the weak global handles in the context list would just become empty when the cycle involving Context/ContextifyContext is unreachable and will get purged sooner or later. So this no longer depend on #56534

[legendecas]

The failed test-inspector-contexts seems to be persistent and could be related

[joyeecheung]

That test seems to be doing something pretty unreliable - calling gc() in a loop until a context is collected. It should at least do that with setImmediate to free up the main thread, but we also already know from all the workarounds in test/common/gc that there is no reliable way to count on one particular object to be garbage collected (modulo taking a heap snapshot but that's a hack that I prefer not to abuse). We can see if using setImmediate helps and if not, try allocating a ton of contexts and see if any of them would be destroyed. If none of it helps stabilizing the test maybe we should just remove it or mark it as flaky...

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64519/

[jasnell]

Love the additional comment detail :-)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64522/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/64535/

[nodejs-github-bot]

Landed in cee63dc...74717cb

[aduh95]

I had to drop 74717cb from v22.x-staging as it was making the build fail. Manually backport would be necessary if we want it there

In file included from ../../src/module_wrap.cc:5:
../../src/node_contextify.h:75:33: error: expected class name
   75 | class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) {
      |                                 ^
../../src/node_contextify.h:77:3: error: unknown type name 'SET_CPPGC_NAME'
   77 |   SET_CPPGC_NAME(ContextifyContext)
      |   ^
../../src/node_contextify.h:77:36: error: expected ';' at end of declaration list
   77 |   SET_CPPGC_NAME(ContextifyContext)
      |                                    ^
      |                                    ;
../../src/node_contextify.h:106:25: error: use of undeclared identifier 'env'
  106 |     return context_.Get(env()->isolate());