Skip to content

Commit de1e9ae

Browse files
committed
docs: add memory leak flag investigation documentation
Document findings from investigating FF_LSDV_4620_3_ML and FF_DEV_3391 feature flags: - FF_LSDV_4620_3_ML is explicitly disabled due to LSE/LSO React build differences, causing memory leaks in V17 destroy path - FF_DEV_3391 controls per-annotation config trees for View All mode - Includes mitigation strategies that work without enabling flags - Provides testing recommendations for re-enabling the flag
1 parent 609d271 commit de1e9ae

1 file changed

Lines changed: 122 additions & 0 deletions

File tree

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Memory Leak Flag Investigation
2+
3+
## Overview
4+
5+
This document summarizes the investigation into memory-related feature flags in the Label Studio Editor:
6+
- `FF_LSDV_4620_3_ML` - Memory leak fixes
7+
- `FF_DEV_3391` - Per-annotation config trees
8+
9+
## FF_LSDV_4620_3_ML (Memory Leak Fixes)
10+
11+
### Current Status: **DISABLED by default**
12+
13+
The flag is explicitly overridden to `false` in `utils/feature-flags.ts`:
14+
15+
```typescript
16+
const override: Record<string, boolean> = {
17+
fflag_fix_front_lsdv_4620_memory_leaks_100723_short: false,
18+
};
19+
```
20+
21+
With this comment:
22+
> TODO: remove the override + if statement once LSE and LSO start building react the same way
23+
24+
### What the Flag Controls
25+
26+
In `LabelStudio.tsx` (V17 destroy path), when **enabled**:
27+
28+
1. `clearRenderedApp()` - Properly unmounts React before MST destruction
29+
2. `this.store.selfDestroy()` - Walks MST tree and destroys children in order
30+
3. Nulls out `this.store`, `this.destroy`, removes from `LabelStudio.instances`
31+
32+
When **disabled** (current default):
33+
34+
1. ❌ React is NOT unmounted before MST destruction
35+
2. ❌ MST children are not destroyed in order
36+
3. ❌ References are not nulled out → **Memory leaks**
37+
38+
### Why It's Disabled
39+
40+
The comment indicates there's an incompatibility between how:
41+
- Label Studio Enterprise (LSE)
42+
- Label Studio Open Source (LSO)
43+
44+
...build React. The specific issue is not documented, but likely relates to:
45+
- Different React versions (17 vs 18)
46+
- Different bundling configurations
47+
- Race conditions in the unmount/destroy sequence
48+
49+
### Recommended Actions
50+
51+
1. **Test enabling the flag** in LSO-only environment to document exact failures
52+
2. **Identify the React build difference** between LSE and LSO
53+
3. **Fix root cause** rather than keeping the flag disabled
54+
4. Alternatively, implement flag-independent cleanup for critical paths:
55+
- Always clear `window.Htx` (prevents store retention)
56+
- Always null out `this.store` after destruction
57+
- Add `isAlive` guards to all async operations
58+
59+
## FF_DEV_3391 (Per-Annotation Config Trees)
60+
61+
### Status: Feature flag for "View All" mode
62+
63+
This flag enables separate config trees per annotation, required for interactive annotations in View All mode.
64+
65+
### Impact on Memory
66+
67+
When enabled:
68+
- Each annotation gets its own config tree copy
69+
- Memory scales linearly with annotation count
70+
- Helps prevent cross-annotation state pollution
71+
72+
When disabled:
73+
- Single shared config tree
74+
- Lower memory usage
75+
- View All mode is view-only
76+
77+
### Location
78+
79+
Used in `stores/Annotation/Annotation.js` to conditionally create per-annotation root trees.
80+
81+
## Memory Leak Mitigation Without Flags
82+
83+
Even with flags disabled, these patterns help reduce leaks:
84+
85+
### 1. Clear Global References
86+
87+
```javascript
88+
// Always do this in destroy(), regardless of flags
89+
window.Htx = null;
90+
```
91+
92+
### 2. Guard Async Operations
93+
94+
```javascript
95+
setTimeout(() => {
96+
if (!isAlive(self)) return; // Guard against destroyed store
97+
// ... rest of logic
98+
});
99+
```
100+
101+
### 3. Dispose Subscriptions
102+
103+
```javascript
104+
let disposer = onSnapshot(self.areas, callback);
105+
// In beforeDestroy:
106+
disposer?.();
107+
```
108+
109+
### 4. Use Safe References
110+
111+
Where possible, use `types.safeReference()` instead of `types.reference()` to avoid warnings during destruction.
112+
113+
## Testing Recommendations
114+
115+
1. Enable `FF_LSDV_4620_3_ML` in test environment
116+
2. Navigate in/out of labeling interface
117+
3. Monitor:
118+
- Memory heap snapshots
119+
- Console warnings
120+
- JS errors
121+
4. Document exact failure conditions
122+
5. Fix root causes before re-enabling

0 commit comments

Comments
 (0)