Skip to content

[Vine/Server] ActiveOperations map leaks CancellationTokens — UnregisterOperation never called in process_cocoon_request #63

@NikolaRHristov

Description

@NikolaRHristov

Problem

MountainVinegRPCService::process_cocoon_request calls RegisterOperation at the start of every RPC but never calls UnregisterOperation on the completion path (success or error). The ActiveOperations: Arc<RwLock<HashMap<u64, CancellationToken>>> map therefore grows unboundedly for the lifetime of the process — one leaked Arc-backed CancellationToken per RPC call.

In long sessions with 14,000+ RPCs this is 14,000+ live entries in a map that is never pruned.

File: Source/Vine/Server/MountainVinegRPCService.rs

Root Cause

// RegisterOperation is called unconditionally...
let token = self.RegisterOperation(RequestIdentifier).await;

// ...but UnregisterOperation is never called in the dispatch path:
match DispatchResult {
    Ok(_) => { /* no unregister */ },
    Err(_) => { /* no unregister */ },
}

Additionally, cancel_operation fires token.cancel() but also does not remove the entry, relying on the handler to call UnregisterOperation — which never happens.

Fix

  • Only register operations that explicitly opt into cancellation (those that will call UnregisterOperation themselves from the handler body).
  • Remove the unconditional RegisterOperation call from process_cocoon_request.
  • In cancel_operation: remove the entry from ActiveOperations immediately after token.cancel().
  • Add UnregisterOperation calls to both the success and error arms of process_cocoon_request for any operation that did register.

Additional Scope for Same PR

  • The write() lock on ActiveOperations is acquired on every RPC. With 50 concurrent RPCs this serialises all registrations through a single writer queue. Remove the unconditional registration to eliminate the contention entirely.
  • ValidateRequest rejects method names containing "::" or "../" — both checks block legitimate callers and provide no real security (path traversal is in parameters, not method names). Remove both checks.
  • The hot-RPC instrumentation path calls SystemTime::now() unconditionally for $tree:register, Configuration.Inspect, and Command.Execute. Gate behind a LAND_VINE_LATENCY_TRACE env flag check.
  • Reorder the top 3 highest-frequency send_cocoon_notification match arms (progress.report, extensionHostMessage, outputChannel.append) to the top of the match to reduce average comparison count from ~40 to ~3.

Metadata

Metadata

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions