fix: add destroy methods to prevent memory leaks in protocol clients

[wolfsilver]

In the event of prolonged network outages, this may lead to severe memory issues.

This pull request introduces a resource cleanup mechanism across the agent protocol and its gRPC client implementations to prevent memory leaks and ensure graceful shutdown. The main changes add a standardized optional destroy() method to key interfaces and classes, and implement cleanup logic for timers, event listeners, and buffers.

Resource cleanup and lifecycle management:

• Added an optional destroy() method to the Protocol and Client interfaces (Protocol.ts, Client.ts), and implemented it in GrpcProtocol, HeartbeatClient, and TraceReportClient to clean up resources such as timers, event listeners, and buffers.

Memory leak prevention in gRPC clients:

• In HeartbeatClient, implemented destroy() to clear the heartbeat timer and log cleanup.
• In TraceReportClient, refactored event listener registration to store a reference for proper removal, and implemented destroy() to remove the event listener, clear timeouts and buffers, and log cleanup. Also added logic to limit buffer size and discard oldest segments with a warning to prevent memory leaks during network issues.

Agent shutdown support:

• Added a destroy() method to the Agent class to call the protocol's destroy() method, reset protocol state, and log the shutdown process.

[a526672351]

已收到！

[tkNobug]

这是来自QQ邮箱的假期自动回复邮件。您好，我最近正在休假中，无法亲自回复您的邮件。我将在假期结束后，尽快给您回复。

[Copilot]

Pull request overview

This pull request introduces a resource cleanup mechanism to prevent memory leaks in the SkyWalking Node.js agent by adding destroy() methods across the protocol layer. The changes address potential memory issues during prolonged network outages by properly cleaning up timers, event listeners, buffers, and implementing buffer size limits.

Changes:

• Added optional destroy() method to Protocol and Client interfaces for standardized cleanup
• Implemented resource cleanup in HeartbeatClient (timer), TraceReportClient (timer, event listener, buffer), and GrpcProtocol
• Added buffer size limiting in TraceReportClient to prevent unbounded memory growth during network issues
• Added destroy() method to the Agent class for graceful shutdown

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/agent/protocol/Protocol.ts	Added optional destroy() method to Protocol interface
src/agent/protocol/grpc/clients/Client.ts	Added optional destroy() method to Client interface
src/agent/protocol/grpc/GrpcProtocol.ts	Implemented destroy() to clean up both heartbeat and trace report clients
src/agent/protocol/grpc/clients/HeartbeatClient.ts	Implemented destroy() to clear heartbeat interval timer
src/agent/protocol/grpc/clients/TraceReportClient.ts	Implemented destroy() to clean up event listener, timeout, and buffer; added buffer size limiting to prevent memory leaks
src/index.ts	Added destroy() method to Agent class to trigger protocol cleanup and reset state

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The gRPC client channels should be closed when destroying the client to properly release network resources. The gRPC library provides a close() method on the client to gracefully shutdown the channel. Consider adding this.reporterClient.close() after clearing the timeout and before clearing the buffer to ensure all network connections are properly terminated.

[Copilot]

The gRPC client channel should be closed when destroying the client to properly release network resources. The gRPC library provides a close() method on the client to gracefully shutdown the channel. Consider adding this.managementServiceClient.close() after clearing the heartbeat timer to ensure all network connections are properly terminated.

Suggested change

	// Close gRPC client to release underlying network resources
	this.managementServiceClient.close();

[Copilot]

There's a potential race condition if destroy() is called while reportFunction() is executing. The timeout could be cleared while the report is in progress, but then reportFunction() might schedule a new timeout at line 106 after destroy has completed. Consider adding a flag to track if the client is being destroyed and check it before scheduling new timeouts in reportFunction().

[Copilot]

There's a potential race condition if destroy() is called while segments are being reported. The buffer could be cleared at line 139 while reportFunction() is iterating over it (lines 91-99), potentially causing data loss or unexpected behavior. Consider either waiting for in-flight reports to complete before cleanup, or adding synchronization to prevent concurrent access to the buffer.

[Copilot]

Consider calling flush() before destroying the protocol to ensure any pending segments are reported to the backend before cleanup. This would prevent potential data loss of in-flight or buffered trace segments during shutdown. The flush could be made asynchronous to allow graceful completion of pending reports.

[Copilot]

The null check if (this.segmentFinishedListener) is unnecessary since the listener is always assigned in the constructor and never set to null or undefined elsewhere in the code. This check can be safely removed for cleaner code.

Suggested change

	if (this.segmentFinishedListener) {
	emitter.off('segment-finished', this.segmentFinishedListener);
	}
	emitter.off('segment-finished', this.segmentFinishedListener);

[Copilot]

The destroy() method should be idempotent to handle cases where it's called multiple times. Consider adding a flag to track if the client has already been destroyed and return early if it's called again. This prevents potential errors from clearing already-cleared resources and removes event listeners multiple times.

[Copilot]

The destroy() method should be idempotent to handle cases where it's called multiple times. Consider adding a flag to track if the client has already been destroyed and return early if it's called again. This prevents potential errors from clearing already-cleared resources.