Skip to content

Fix memory leaks (#616)#816

Merged
objective-see merged 1 commit intoobjective-see:masterfrom
jhult:fix/memory-leaks-616
Feb 24, 2026
Merged

Fix memory leaks (#616)#816
objective-see merged 1 commit intoobjective-see:masterfrom
jhult:fix/memory-leaks-616

Conversation

@jhult
Copy link
Copy Markdown
Contributor

@jhult jhult commented Dec 15, 2025 •
edited
Loading

Fix Memory Leaks in System Extension

Fixes #616, where 1+ memory leak(s) in the system extension could cause memory to grow from ~100 MB to 10+ GB, causing network failures.

Note: This was largerly generated by Claude. I ran many passes to try to get the best possible outcome. However, since I can't sign the extension and can't test, I can't vouch for whether this actually works (or is even the proper approach).

Problem

The system extension accumulates memory over time (versions 2.4.3 - 3.1.5), eventually consuming 10+ GB RAM and breaking network connectivity.

Root causes

  1. Retain cycles in XPC reply blocks
  2. Flows never released from pendingAlerts/relatedFlows dictionaries
  3. Flows not resumed before removal (bug in processRelatedFlow)
  4. Orphaned flows not cleaned up when processes exit
  5. Flows not cleaned up on extension shutdown

Solution

  1. Break Retain Cycles
    A. Use weak-strong dance pattern in alert reply blocks
    B. Prevents self from being retained by async XPC callbacks

  2. Track All Flows
    A. Add pendingAlerts dictionary to track flows awaiting user response
    B. Ensures flows aren't lost when user doesn't respond

  3. Fix processRelatedFlow Bug
    A. Resume flows BEFORE removing from dictionary
    B. Handle pause verdict correctly (keep in dictionary for next round)

  4. Cleanup Orphaned Flows
    A. Add isFlowOrphaned helper and cleanupOrphanedFlows method
    B. Resume orphaned flows with dropVerdict (critical for system to release)
    C. Triggers: every 5 minutes + on XPC failure

  5. Dealloc Cleanup
    A. Resume all flows during dealloc using resumeFlows helper
    B. Prevents flows from remaining paused on extension shutdown

  6. Race Condition Prevention
    A. Check if flow still in pendingAlerts before processing user response
    B. Prevents double-resume if cleanup already handled flow
    C. Ensures user's choice isn't applied to stale flows

All flow resume operations use shared resumeFlows helper to eliminate code duplication and ensure consistent error handling.

Testing

Unit tests

  • Passive mode tests: 9/9
  • Memory leak tests: 10/10

Run tests: cd LuLu/Tests && bash run_tests.sh

Integration testing

Requires system extension runtime (which I can't sign and thus cannot run/test).

# Cleanup activity
log stream --level debug --predicate 'subsystem == "com.objective-see.lulu" && composedMessage CONTAINS "cleanup"'

# Memory usage
watch -n 60 'ps aux | grep lulu.extension | awk "{print \$6/1024 \" MB\"}"'
Expected behavior
  • Cleanup runs every 5 minutes
  • Memory stays < 200 MB (no continuous growth)
  • Network remains functional

Edge Cases Handled

  1. Process exits before alert response (timer cleanup)
  2. XPC connection fails (immediate cleanup)
  3. User never responds (timer cleanup)
  4. High volume short-lived processes (periodic cleanup)
  5. Extension shutdown with pending flows (dealloc cleanup)
  6. Process exits during user response window (race prevention)
  7. Pause verdict during processing (flow kept in dictionary)
  8. Concurrent dictionary access (all @synchronized)

@jhult
Copy link
Copy Markdown
Contributor Author

jhult commented Dec 18, 2025

@objective-see, when you get a chance, I'd appreciate your review.

@jhult
Copy link
Copy Markdown
Contributor Author

jhult commented Jan 7, 2026

@objective-see, any chance you've had some time to review?

@j-tamad
Copy link
Copy Markdown

j-tamad commented Jan 7, 2026 via email

@objective-see
Copy link
Copy Markdown
Owner

objective-see commented Jan 7, 2026

@jhult: thanks will take a look ASAP!

@j-tamad: If I view your profile, I can see you're following Objective-See (I clicked on "following"):
Screenshot 2026-01-06 at 21 17 14
Screenshot 2026-01-06 at 21 17 25

I'd start by unfollowing, as maybe that'd "unsubscribe" you?

@j-tamad
Copy link
Copy Markdown

j-tamad commented Jan 7, 2026 via email

@objective-see
Copy link
Copy Markdown
Owner

objective-see commented Jan 7, 2026

@j-tamad you have to unfollow! It's not something I can do 🙈

@jhult
Copy link
Copy Markdown
Contributor Author

jhult commented Jan 19, 2026

@objective-see, did you get a chance to review?

@objective-see
Copy link
Copy Markdown
Owner

objective-see commented Jan 31, 2026

Thanks for the PR 🙏🏽
...but I'm not seeing how this fixes any (common) memory leaks, plus its a ton of (AI?) generated code, that adds a lot of complexity and stuff that IMHO is unnecessary.

For example, related flows are only saved when an alert (for the same process is successfully delivered to the user). When they response, all related flows are removed and then handled. So even if the handling fails, the flow has been removed (freed). So the whole cleanupOrphanedFlows seems moot.

Also, saving all pending flows (to resume if extension is shutting down), actually increased the memory footprint. An I'm really ok in the (rare) case the user shutdowns the extension that any paused flows are then terminated (i.e. current logic seems fine).

I also asked Claude about retaining self in the alert blocks and it told me the current code is fine 🤷‍♂️

I'll spend more time looking through this though!

P.S. if you disable SIP and AMFI you should be able to build the extension yourself (if you have a Developer ID).

@objective-see objective-see mentioned this pull request Feb 17, 2026
Open
@jhult
fix: resume related flows before removing from array
ead1dab 
Related flows were removed from the tracking array but never resumed via resumeFlow:withVerdict:, causing them to stay paused indefinitely and leak memory.

Fixes objective-see#616
@jhult jhult force-pushed the fix/memory-leaks-616 branch from a0b3463 to ead1dab Compare February 20, 2026 16:37
@jhult
Copy link
Copy Markdown
Contributor Author

jhult commented Feb 21, 2026

My initial commit in this PR was over-engineered (and was in fact written by AI). Here's a focused replacement (also written by AI).

"I'm not seeing how this fixes any (common) memory leaks"

The actual bug is in processRelatedFlow:. When a user responds to an alert, related flows for that process are re-evaluated via processEvent:. But processEvent: only returns a verdict — it was designed for new flows where the OS uses the return value. For already-paused related flows, nobody calls resumeFlow:withVerdict:, so they stay paused forever and the OS never releases them.

For chatty processes (e.g. browsers, VPN clients), dozens or hundreds of related flows queue up while the user decides. Every one of them leaks. Over hours/days this reaches gigabytes.

For chatty processes (browsers, VPN clients), this means hundreds of leaked flows per alert interaction.

"the whole cleanupOrphanedFlows seems moot"

Correct. With the actual bug fixed, flows are properly resumed when the user responds, so there's nothing to clean up.

"saving all pending flows actually increased the memory footprint"

Correct. Removed entirely.

"I also asked Claude about retaining self in the alert blocks and it told me the current code is fine"

Agreed. The XPC framework owns the block, not self. It's not a retain cycle. Removed the weak/strong dance.

The fix

One method changed: processRelatedFlow: (~20-25 lines). Resume the flow before removing it from the array.

flow = flows[i];
verdict = [self processEvent:flow];       // get verdict first
if([NEFilterNewFlowVerdict pauseVerdict] == verdict)
{
    break;                                // new alert shown, wait for user
}
[self resumeFlow:flow withVerdict:verdict]; // resume the paused flow
[flows removeObjectAtIndex:i];             // then remove from tracking

Also cleans up empty entries from relatedFlows dictionary after all flows for a process are handled.

What this does NOT change

  • No new properties or instance variables
  • No timers or periodic cleanup
  • No changes to the alert: method or XPC handling
  • No dealloc override

objective-see
objective-see requested changes Feb 22, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@objective-see objective-see left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks great, save for one (potential?) issue.

I think the code should use isEqual: instead of ==:

Fix:

verdict = [self processEvent:flow];
if([verdict isEqual:[NEFilterNewFlowVerdict pauseVerdict]])

Otherwise I think == is just doing a pointer comparison (unless Apple's implementation does return a singleton for pauseVerdict, which would make == work in practice). Either way isEqual: is the safer/"correct" approach (this was wrong in the original code too)

@objective-see objective-see merged commit 13f4f0c into objective-see:master Feb 24, 2026
@jhult jhult deleted the fix/memory-leaks-616 branch February 24, 2026 21:24
@objective-see
Copy link
Copy Markdown
Owner

objective-see commented Mar 14, 2026

Now in latest release:
https://github.com/objective-see/LuLu/releases/tag/v4.3.0

@jhult if you continue to see memory issues, please let me know! I have a bit more time on my hands these days for fixes 🙏🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@objective-see objective-see objective-see approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory leak?

3 participants

@jhult @j-tamad @objective-see