Skip to content

Commit a070f5e

Browse files
NickGerlemanmeta-codesync[bot]
authored andcommitted
Replace manual memory management with FinalizationRegistry (#1908)
Summary: Pull Request resolved: #1908 Fixes #1818 Fixes #1572 Yoga's JavaScript API currently requires users to manually call `node.free()`, `node.freeRecursive()`, or `config.free()` to release WASM memory. This is error-prone and unusual for a JavaScript library. This diff uses `FinalizationRegistry` (available in all modern JS engines) to automatically free WASM memory when JS objects are garbage collected, removing this footgun entirely. Changes: - Added `FinalizationRegistry` instances for `Node` and `Config` in `wrapAssembly.ts`. When a `NodeImpl`/`ConfigImpl` is constructed, it is registered with the appropriate registry. The weak reference callback receives the WASM pointer and calls the appropriate C free function. - The Node finalizer calls `YGNodeFinalize` (not `YGNodeFree`) to safely deallocate without disconnecting nodes from their owner/children, since an entire tree may be garbage collected together. - Removed `free()` and `freeRecursive()` from the public `Node` type and `free()` from the public `Config` type. - Removed `Yoga.Node.destroy()` and `Yoga.Config.destroy()` factory methods. - Fixed `setDirtiedFunc` to use `WeakRef(this)` instead of capturing `this` directly in the closure stored in the dirtied func Map. The previous `() => dirtiedFunc(this)` pattern created a strong reference from the Map to the Node, preventing GC of detached nodes with a dirtied func set. - Updated `gentest-javascript.js` to stop generating `try`/`finally` cleanup blocks and `let root` declarations in test prologues/epilogues. - Removed all `free()`/`freeRecursive()`/`config.free()` calls from 25 generated test files and 9 hand-written test files. - Re-signed all generated test files with signedsource. - Updated `README.md` to remove manual memory management documentation. Reviewed By: cipolleschi Differential Revision: D95014519
1 parent 8d88a4a commit a070f5e

40 files changed

Lines changed: 37665 additions & 43088 deletions

gentest/gentest-javascript.js

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ JavascriptEmitter.prototype = Object.create(Emitter.prototype, {
5959
this.push(`${testFn}('${name}', () => {`);
6060
this.pushIndent();
6161
this.push('const config = Yoga.Config.create();');
62-
this.push('let root;');
6362
this.push('');
6463

6564
if (experiments.length > 0) {
@@ -70,39 +69,17 @@ JavascriptEmitter.prototype = Object.create(Emitter.prototype, {
7069
}
7170
this.push('');
7271
}
73-
74-
this.push('try {');
75-
this.pushIndent();
7672
},
7773
},
7874

7975
emitTestTreePrologue: {
8076
value: function (nodeName) {
81-
if (nodeName === 'root') {
82-
this.push(`root = Yoga.Node.create(config);`);
83-
} else {
84-
this.push(`const ${nodeName} = Yoga.Node.create(config);`);
85-
}
77+
this.push(`const ${nodeName} = Yoga.Node.create(config);`);
8678
},
8779
},
8880

8981
emitTestEpilogue: {
9082
value: function (_experiments) {
91-
this.popIndent();
92-
this.push('} finally {');
93-
this.pushIndent();
94-
95-
this.push("if (typeof root !== 'undefined') {");
96-
this.pushIndent();
97-
this.push('root.freeRecursive();');
98-
this.popIndent();
99-
this.push('}');
100-
this.push('');
101-
this.push('config.free();');
102-
103-
this.popIndent();
104-
this.push('}');
105-
10683
this.popIndent();
10784
this.push('});');
10885
},

javascript/README.md

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,6 @@ const node = Yoga.Node.create();
1313
node.setAlignContent(Align.Center);
1414
```
1515

16-
Objects created by `Yoga.<>.create()` are not automatically garbage collected and should be freed once they are no longer in use.
17-
18-
```ts
19-
// Free a config
20-
config.free();
21-
22-
// Free a tree of Nodes
23-
node.freeRecursive();
24-
25-
// Free a single Node
26-
node.free();
27-
```
28-
2916
## Requirements
3017

3118
`yoga-layout` requires a toolchain that supports ES Modules and top-level await.

javascript/src/wrapAssembly.ts

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ type Value = {
4848
};
4949

5050
export type Config = {
51-
free(): void;
5251
isExperimentalFeatureEnabled(feature: ExperimentalFeature): boolean;
5352
setExperimentalFeatureEnabled(
5453
feature: ExperimentalFeature,
@@ -77,8 +76,6 @@ export type Node = {
7776
direction?: Direction,
7877
): void;
7978
copyStyle(node: Node): void;
80-
free(): void;
81-
freeRecursive(): void;
8279
getAlignContent(): Align;
8380
getAlignItems(): Align;
8481
getAlignSelf(): Align;
@@ -263,13 +260,11 @@ export type Node = {
263260
export type Yoga = {
264261
Config: {
265262
create(): Config;
266-
destroy(config: Config): void;
267263
};
268264
Node: {
269265
create(config?: Config): Node;
270266
createDefault(): Node;
271267
createWithConfig(config: Config): Node;
272-
destroy(node: Node): void;
273268
};
274269
} & typeof YGEnums;
275270

@@ -365,16 +360,24 @@ export default function wrapAssembly(lib: any): Yoga {
365360
}
366361
}
367362

363+
// --- FinalizationRegistry for automatic cleanup ---
364+
const configRegistry = new FinalizationRegistry((ptr: number) => {
365+
lib._YGConfigFree(ptr);
366+
});
367+
368+
const nodeRegistry = new FinalizationRegistry((ptr: number) => {
369+
lib._yogaMeasureFuncs.delete(ptr);
370+
lib._yogaDirtiedFuncs.delete(ptr);
371+
lib._YGNodeFinalize(ptr);
372+
});
373+
368374
// --- Config class ---
369375
class ConfigImpl {
370376
_ptr: number;
371377

372378
constructor(ptr: number) {
373379
this._ptr = ptr;
374-
}
375-
376-
free(): void {
377-
lib._YGConfigFree(this._ptr);
380+
configRegistry.register(this, ptr, this);
378381
}
379382

380383
setExperimentalFeatureEnabled(
@@ -423,6 +426,7 @@ export default function wrapAssembly(lib: any): Yoga {
423426
this._ptr = ptr;
424427
this._children = [];
425428
this._parent = null;
429+
nodeRegistry.register(this, ptr, this);
426430
}
427431

428432
// --- Tree hierarchy ---
@@ -454,31 +458,6 @@ export default function wrapAssembly(lib: any): Yoga {
454458
}
455459

456460
// --- Lifecycle ---
457-
free(): void {
458-
lib._yogaMeasureFuncs.delete(this._ptr);
459-
lib._yogaDirtiedFuncs.delete(this._ptr);
460-
461-
// Clear JS tree references
462-
if (this._parent) {
463-
const idx = this._parent._children.indexOf(this);
464-
if (idx !== -1) {
465-
this._parent._children.splice(idx, 1);
466-
}
467-
this._parent = null;
468-
}
469-
this._children = [];
470-
471-
lib._YGNodeFree(this._ptr);
472-
}
473-
474-
freeRecursive(): void {
475-
// Walk children in JS, calling freeRecursive on each
476-
while (this._children.length > 0) {
477-
this._children[0].freeRecursive();
478-
}
479-
this.free();
480-
}
481-
482461
reset(): void {
483462
lib._yogaMeasureFuncs.delete(this._ptr);
484463
lib._yogaDirtiedFuncs.delete(this._ptr);
@@ -920,7 +899,11 @@ export default function wrapAssembly(lib: any): Yoga {
920899

921900
setDirtiedFunc(dirtiedFunc: DirtiedFunction | null): void {
922901
if (dirtiedFunc) {
923-
lib._yogaDirtiedFuncs.set(this._ptr, () => dirtiedFunc(this));
902+
const nodeWeakRef = new WeakRef(this);
903+
lib._yogaDirtiedFuncs.set(this._ptr, () => {
904+
const node = nodeWeakRef.deref();
905+
if (node) dirtiedFunc(node);
906+
});
924907
lib._jswrap_YGNodeSetDirtiedFunc(this._ptr);
925908
} else {
926909
this.unsetDirtiedFunc();
@@ -1011,9 +994,6 @@ export default function wrapAssembly(lib: any): Yoga {
1011994
create(): Config {
1012995
return new ConfigImpl(lib._YGConfigNew());
1013996
},
1014-
destroy(config: Config): void {
1015-
(config as ConfigImpl).free();
1016-
},
1017997
},
1018998
Node: {
1019999
create(config?: Config): Node {
@@ -1032,9 +1012,6 @@ export default function wrapAssembly(lib: any): Yoga {
10321012
lib._YGNodeNewWithConfig((config as ConfigImpl)._ptr),
10331013
);
10341014
},
1035-
destroy(node: Node): void {
1036-
(node as NodeImpl).free();
1037-
},
10381015
},
10391016
...YGEnums,
10401017
};

javascript/tests/Benchmarks/YGBenchmark.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ YGBENCHMARK('Stack with flex', () => {
2727
}
2828

2929
root.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
30-
root.freeRecursive();
3130
});
3231

3332
YGBENCHMARK('Align stretch in undefined axis', () => {
@@ -43,7 +42,6 @@ YGBENCHMARK('Align stretch in undefined axis', () => {
4342
}
4443

4544
root.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
46-
root.freeRecursive();
4745
});
4846

4947
YGBENCHMARK('Nested flex', () => {
@@ -67,7 +65,6 @@ YGBENCHMARK('Nested flex', () => {
6765
}
6866

6967
root.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
70-
root.freeRecursive();
7168
});
7269

7370
YGBENCHMARK('Huge nested layout', () => {
@@ -110,5 +107,4 @@ YGBENCHMARK('Huge nested layout', () => {
110107
}
111108

112109
root.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
113-
root.freeRecursive();
114110
});

0 commit comments

Comments
 (0)