Skip to content

Memory leak potential with JSContext temporary variable #147

Description

@schuyler

Related to: claude/fix-pane-sync-cleaned-up-01CQ6iFLaz5cSgvaqkgQz7ua

Description

The temporary variable used to pass HTML content to JavaScript (line 1111 in MPDocument.m) persists in the JSContext if script evaluation fails before the delete statement executes. This can leak memory for large documents.

Current Code

context[@"window"][@"__macdownTempHtml"] = bodyContent;

NSString *updateScript = [NSString stringWithFormat:
    @"(function(){"
    @"  var scrollY = %.0f;"
    @"  var html = window.__macdownTempHtml;"
    @"  delete window.__macdownTempHtml;"  // <-- Cleanup inside JS
    @"  var body = document.body;"
    @"  body.innerHTML = html;"
    // ... rest of script
    @"})();",
    scrollBefore];

[context evaluateScript:updateScript];

Issues

  1. Cleanup is inside JavaScript - If evaluation fails, cleanup never happens
  2. No error handling - Failed evaluation leaves variable in global scope
  3. Memory accumulation - Large documents can cause significant memory leak over time
  4. No defensive cleanup - Even successful execution should verify cleanup

Leak Scenario

1. User edits large document (bodyContent = 500KB)
2. bodyContent stored in window.__macdownTempHtml
3. JavaScript evaluation fails (syntax error, exception, etc.)
4. Variable persists in JSContext indefinitely
5. Next edit adds another 500KB
6. Memory usage grows with each failed evaluation

Recommended Fixes

Option 1: Cleanup in Objective-C (Recommended)

context[@"window"][@"__macdownTempHtml"] = bodyContent;
[context evaluateScript:updateScript];
// Always cleanup, even if script failed
[context evaluateScript:@"delete window.__macdownTempHtml;"];

Option 2: Try-Finally in JavaScript

@"(function(){"
@"  try {"
@"    var html = window.__macdownTempHtml;"
@"    // ... processing ..."
@"  } finally {"
@"    delete window.__macdownTempHtml;"
@"  }"
@"})();"

Option 3: Use unique temporary names

NSString *tempVar = [NSString stringWithFormat:@"__macdown_temp_%lld", (long long)([[NSDate date] timeIntervalSince1970] * 1000)];
context[@"window"][tempVar] = bodyContent;
// Less likely to conflict, but still needs cleanup

Impact

  • Severity: Important
  • User Impact: Memory leak with failed script evaluations
  • Frequency: Rare (script evaluations usually succeed)
  • Memory Impact: Proportional to document size, can accumulate over long editing sessions

Testing Recommendations

  1. Monitor memory usage during long editing sessions
  2. Inject script evaluation failures to test cleanup
  3. Verify cleanup with large documents (> 1MB)
  4. Profile memory usage with Instruments

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingmemory-managementv0.1Must have for v0.1 release

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions