Skip to content

Conversation

@takaokouji
Copy link

Summary

This PR fixes a critical Maximum call stack size exceeded error in the meshV2 extension when global variables are updated frequently.

Problem

In rate-limiter.js, when merging data into an existing queue item, the resolve and reject callbacks were wrapped with the previous callbacks. Repeated merging led to deep nesting of these functions, eventually exceeding the stack limit.

Solution

  • Managed resolve/reject callbacks in arrays (resolvers and rejecters) instead of nesting them.
  • Optimized logging by using log.debug for high-frequency operations (adding to queue, merging data, sending request).
  • Added periodic statistics reporting (every 10 seconds) for sent and merged items to provide visibility without console flooding.

Test Coverage

  • Added a reproduction unit test test/unit/scratch3_mesh_v2_rate_limiter_repro.js which confirmed the stack overflow before the fix and passed after the fix.
  • Verified that existing rate_limiter.js unit tests still pass.

Addresses smalruby/smalruby3-gui#499

Co-Authored-By: Gemini noreply@google.com

takaokouji and others added 2 commits January 2, 2026 16:22
- Use arrays to manage resolve/reject callbacks instead of nesting functions
- Optimize logging by using log.debug for high-frequency operations
- Added periodic statistics reporting for sent and merged items
- Added reproduction test case

Addresses smalruby/smalruby3-gui#499

Co-Authored-By: Gemini <noreply@google.com>
- Changed log.info to log.debug in sendData to reduce console flooding

Co-Authored-By: Gemini <noreply@google.com>
@takaokouji
Copy link
Author

Code Review Summary ✅

Excellent implementation! This PR perfectly addresses the critical stack overflow issue.

✅ Implementation Quality

1. Array-based Callback Management (rate-limiter.js:79-96)

// ✅ Correct implementation
if (!queueItem.resolvers) {
    queueItem.resolvers = [queueItem.resolve];
    queueItem.rejecters = [queueItem.reject];
    
    queueItem.resolve = result => {
        queueItem.resolvers.forEach(r => r(result));
    };
    queueItem.reject = error => {
        queueItem.rejecters.forEach(r => r(error));
    };
}

queueItem.resolvers.push(resolve);
queueItem.rejecters.push(reject);

Why this works:

  • First merge: converts callbacks to arrays
  • Subsequent merges: O(1) append operation
  • No function nesting → no stack growth
  • Stack depth = O(1) regardless of merge count

2. Logging Optimization

  • ✅ High-frequency logs: log.infolog.debug
  • ✅ Statistics reporting: every 10 seconds
  • ✅ Reduces console flooding: ~30-40 logs/sec → 0 in production

3. Test Coverage

  • ✅ Reproduction test: 15,000 merges
  • ✅ Timeout: 60 seconds (sufficient)
  • ✅ Clear pass/fail criteria

📊 Performance Analysis

Before fix:

  • Stack depth = O(n) where n = merge count
  • 15,000 merges → stack overflow

After fix:

  • Stack depth = O(1)
  • 15,000 merges → array with 15,000 elements
  • Memory: ~240 KB (negligible)

✅ Verification Checklist

  • ✅ Stack overflow fixed
  • ✅ Logging optimized
  • ✅ Statistics reporting added
  • ✅ Regression test added
  • ✅ Existing tests pass
  • ✅ Lint checks pass

🚀 Recommendation

Ready to merge! This fix is critical for production stability.


Reviewed-By: Claude Sonnet 4.5 noreply@anthropic.com

@takaokouji takaokouji merged commit c957ad8 into develop Jan 2, 2026
1 check passed
@takaokouji takaokouji deleted the fix/issue-499-mesh-v2-stack-overflow branch January 2, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants