Commit 4ac3ce7
committed
fix: release ObjectManager's JS handles and JNI weak refs at teardown
ObjectManager had no destructor at all. `delete m_objectManager` ran the
implicit one, which destroyed the containers and abandoned everything they
pointed at.
The expensive part is the JNI weak global refs. m_cache holds one per entry, up
to its capacity of 1000, and LRUCache only ever ran its evict callback under
capacity pressure or explicit invalidation -- never at destruction, since it
had no destructor either. So a worker that touched Java objects abandoned its
whole cache when it died. ART's weak-global table is bounded, so this is not
merely a leak: enough worker cycles exhaust it and ART aborts.
The JS side leaked too. Every linked object owns a Persistent<Object>, a
JSInstanceInfo and an ObjectWeakCallbackState, freed only from the GC
finalizer -- and V8 does not run weak callbacks when an isolate is disposed. In
the default `none` marking mode the finalizer additionally re-arms SetWeak
while the Java counterpart is alive, so those wrappers are deliberately
retained and are therefore all still live at teardown.
Split across the two windows teardown actually has:
- ReleaseAllRegistered(), called from DestroyRuntime while the isolate is alive
and locked: clears each wrapper's JsInfo internal field before freeing the
JSInstanceInfo it points at, resets and deletes the Persistent, and releases
m_poJsWrapperFunc.
- ~ObjectManager, reached from ~Runtime once the isolate is gone and while the
thread is still attached to the JVM: clears the LRU cache, which now evicts
through the callback. It touches no v8 handle.
That ordering is not incidental: Persistent::Reset() after Isolate::Dispose
writes into a freed handle table, and the JNI eviction has to happen before
~Runtime drops the com.tns.Runtime global ref, which ObjectManager calls
through.
m_idToObject now maps to ObjectWeakCallbackState* rather than the bare
Persistent*. The state was created and handed to SetWeak but stored nowhere, so
teardown had no way to reach it or the JSInstanceInfo. That also lets
ReleaseJSInstance free the state, which it never did.1 parent 19faa3d commit 4ac3ce7
4 files changed
Lines changed: 90 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
108 | 125 | | |
109 | 126 | | |
110 | 127 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
220 | 220 | | |
221 | 221 | | |
222 | 222 | | |
223 | | - | |
| 223 | + | |
224 | 224 | | |
225 | 225 | | |
226 | 226 | | |
| |||
296 | 296 | | |
297 | 297 | | |
298 | 298 | | |
299 | | - | |
| 299 | + | |
300 | 300 | | |
301 | 301 | | |
302 | 302 | | |
| |||
460 | 460 | | |
461 | 461 | | |
462 | 462 | | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
463 | 507 | | |
464 | 508 | | |
465 | 509 | | |
| |||
473 | 517 | | |
474 | 518 | | |
475 | 519 | | |
476 | | - | |
| 520 | + | |
477 | 521 | | |
| 522 | + | |
478 | 523 | | |
| 524 | + | |
479 | 525 | | |
480 | 526 | | |
481 | 527 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
22 | 37 | | |
23 | 38 | | |
24 | 39 | | |
| |||
201 | 216 | | |
202 | 217 | | |
203 | 218 | | |
204 | | - | |
| 219 | + | |
205 | 220 | | |
206 | 221 | | |
207 | 222 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
983 | 983 | | |
984 | 984 | | |
985 | 985 | | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
| 992 | + | |
| 993 | + | |
986 | 994 | | |
987 | 995 | | |
988 | 996 | | |
| |||
0 commit comments