Commit 7c86624
authored
fix(Worklets): RemoteFunction memory cycle (#9673)
## Summary
Fixes #9661
In #9272 which was supposed to fix memory issues with Remote Function, I
accidentally introduced a memory-leak due to reference retain cycle.
### Before
Basically, RemoteFunctionRegistry would hold a strong reference to the
(remote) function, which in turn would hold a strong reference to the
serialized remote function, which would hold the C++ value.
This C++ value would erase the original function from the registry, but
only in its destructor - which would never happen, because the registry
itself would keep it alive forever.
```mermaid
flowchart TB
subgraph RN[RN Runtime]
R[RemoteFunctionRegistry]
F[remote function]
C[serialized remote function]
end
subgraph CPP[C++ heap]
S[C++ value]
end
subgraph WK[Worker Runtime]
H[holderFunction]
end
R --> F
F -.-> C
C --> S
H --> S
S ==>|"registry.delete(id) — never invoked"| R
linkStyle 0,2,3 stroke:#2f9e44,stroke-width:2px
linkStyle 1 stroke:#868e96,stroke-width:2px
linkStyle 4 stroke:#e03131,stroke-width:3px
```
### After
To fix this issue, I split the C++ Remote Function for RN callbacks into
two subclasses - one would only be owned by the RN Runtime, and the
other one from Worklet Runtimes.
If all Worklet Runtime references would get cleaned up, then they would
schedule cleanup on the RN Runtime C++ side, to erase the original
function from the registry. Then, this would in turn release the cache
value and the remaining C++ value.
```mermaid
flowchart TB
subgraph RN[RN Runtime]
R[RemoteFunctionRegistry]
F[remote function]
C[serialized remote function]
end
subgraph CPP[C++ heap]
O[RN-owned C++ value]
P[Worklet-owned C++ value]
end
subgraph WK[Worker Runtime]
H[holderFunction]
end
R --> F
F -.-> C
C --> O
H --> P
P --> O
O -.-> P
P ==>|"registry.delete(id)"| R
linkStyle 0,2,3,4 stroke:#2f9e44,stroke-width:2px
linkStyle 1,5 stroke:#868e96,stroke-width:2px
linkStyle 6 stroke:#2f9e44,stroke-width:3px
```
## Test Plan
I used the following code and was tracking constructor and destructor
invocations of `SerializableRemoteFunction` in debugger.
<details><summary>Code</summary>
<p>
```tsx
import { useEffect } from 'react';
import { Button, StyleSheet, View } from 'react-native';
import {
scheduleOnRN,
scheduleOnUI,
createWorkletRuntime,
scheduleOnRuntime,
createSerializable,
} from 'react-native-worklets';
const workerRuntime = createWorkletRuntime();
export default function App() {
useEffect(() => {
scheduleOnUI(() => {
function foo() {
const _ = new Array(10000).fill(0);
requestAnimationFrame(foo);
}
foo();
});
});
return (
<View style={styles.container}>
<Button
title="Init runtime"
onPress={() => {
scheduleOnUI(() => {
// scheduleOnRuntime(workerRuntime, () => {
'worklet';
console.log('init');
});
}}
/>
<Button
title="Press me first"
onPress={() => {
function logTest() {
console.log('Button pressed!');
}
// scheduleOnRuntime(workerRuntime, () => {
scheduleOnUI(() => {
'worklet';
scheduleOnRN(logTest);
});
}}
/>
<Button
title="Trigger GC on RN"
onPress={() => {
console.log('Triggering GC on RN');
gc();
}}
/>
<Button
title="Trigger GC on Worker"
onPress={() => {
// scheduleOnRuntime(workerRuntime, () => {
scheduleOnUI(() => {
'worklet';
console.log('Triggering GC on Worker');
gc();
});
}}
/>
</View>
);
}
const styles = StyleSheet.create({
container: {
flex: 1,
backgroundColor: '#fff',
alignItems: 'center',
justifyContent: 'center',
},
});
```
</p>
</details>1 parent 4eda300 commit 7c86624
8 files changed
Lines changed: 275 additions & 143 deletions
File tree
- packages/react-native-worklets/Common/cpp/worklets
- Compat
- NativeModules
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| |||
348 | 349 | | |
349 | 350 | | |
350 | 351 | | |
351 | | - | |
| 352 | + | |
352 | 353 | | |
353 | 354 | | |
354 | 355 | | |
355 | | - | |
| 356 | + | |
356 | 357 | | |
357 | 358 | | |
358 | 359 | | |
| |||
Lines changed: 1 addition & 67 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | 26 | | |
38 | 27 | | |
39 | 28 | | |
| |||
302 | 291 | | |
303 | 292 | | |
304 | 293 | | |
305 | | - | |
306 | | - | |
307 | | - | |
308 | | - | |
309 | | - | |
310 | | - | |
311 | | - | |
312 | | - | |
313 | | - | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | | - | |
324 | | - | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
329 | | - | |
330 | | - | |
331 | | - | |
332 | | - | |
333 | | - | |
334 | | - | |
335 | | - | |
336 | | - | |
337 | | - | |
338 | | - | |
339 | | - | |
340 | | - | |
341 | | - | |
342 | | - | |
343 | | - | |
344 | | - | |
345 | | - | |
346 | | - | |
347 | | - | |
348 | | - | |
349 | | - | |
350 | | - | |
351 | | - | |
352 | | - | |
353 | | - | |
354 | | - | |
355 | | - | |
356 | | - | |
357 | | - | |
358 | | - | |
359 | | - | |
360 | 294 | | |
361 | 295 | | |
362 | 296 | | |
| |||
Lines changed: 1 addition & 65 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
289 | 290 | | |
290 | 291 | | |
291 | 292 | | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
299 | | - | |
300 | | - | |
301 | | - | |
302 | | - | |
303 | | - | |
304 | | - | |
305 | | - | |
306 | | - | |
307 | | - | |
308 | | - | |
309 | | - | |
310 | | - | |
311 | | - | |
312 | | - | |
313 | | - | |
314 | | - | |
315 | | - | |
316 | | - | |
317 | | - | |
318 | | - | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | | - | |
324 | | - | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
329 | | - | |
330 | | - | |
331 | | - | |
332 | | - | |
333 | | - | |
334 | | - | |
335 | | - | |
336 | | - | |
337 | | - | |
338 | | - | |
339 | | - | |
340 | | - | |
341 | | - | |
342 | | - | |
343 | | - | |
344 | | - | |
345 | | - | |
346 | | - | |
347 | | - | |
348 | | - | |
349 | | - | |
350 | | - | |
351 | | - | |
352 | | - | |
353 | | - | |
354 | | - | |
355 | | - | |
356 | | - | |
357 | 293 | | |
358 | 294 | | |
359 | 295 | | |
| |||
Lines changed: 6 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
| |||
60 | 61 | | |
61 | 62 | | |
62 | 63 | | |
63 | | - | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
68 | | - | |
| 69 | + | |
69 | 70 | | |
70 | 71 | | |
71 | 72 | | |
72 | | - | |
| 73 | + | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
77 | | - | |
78 | | - | |
| 78 | + | |
| 79 | + | |
79 | 80 | | |
80 | 81 | | |
81 | 82 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
67 | | - | |
| 66 | + | |
68 | 67 | | |
69 | 68 | | |
70 | 69 | | |
71 | 70 | | |
72 | 71 | | |
73 | | - | |
74 | | - | |
| 72 | + | |
75 | 73 | | |
76 | 74 | | |
77 | 75 | | |
| |||
Lines changed: 113 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
0 commit comments