Skip to content

Commit 2d85ed5

Browse files
authored
fix: add destroy methods to prevent memory leaks in protocol clients (#129)
Co-authored-by: wolfsilver <2452450+wolfsilver@users.noreply.github.com>
1 parent 7103eba commit 2d85ed5

6 files changed

Lines changed: 69 additions & 2 deletions

File tree

src/agent/protocol/Protocol.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,6 @@ export default interface Protocol {
2828
report(): this;
2929

3030
flush(): Promise<any> | null;
31+
32+
destroy?(): void;
3133
}

src/agent/protocol/grpc/GrpcProtocol.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,10 @@ export default class GrpcProtocol implements Protocol {
4747
flush(): Promise<any> | null {
4848
return this.traceReportClient.flush();
4949
}
50+
51+
destroy(): void {
52+
// Clean up both clients to prevent memory leaks
53+
this.heartbeatClient.destroy?.();
54+
this.traceReportClient.destroy?.();
55+
}
5056
}

src/agent/protocol/grpc/clients/Client.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,6 @@ export default interface Client {
2323
start(): void;
2424

2525
flush(): Promise<any> | null;
26+
27+
destroy?(): void;
2628
}

src/agent/protocol/grpc/clients/HeartbeatClient.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,14 @@ export default class HeartbeatClient implements Client {
8989
logger.warn('HeartbeatClient does not need flush().');
9090
return null;
9191
}
92+
93+
destroy(): void {
94+
// Clear heartbeat timer to prevent memory leak
95+
if (this.heartbeatTimer) {
96+
clearInterval(this.heartbeatTimer);
97+
this.heartbeatTimer = undefined;
98+
}
99+
100+
logger.info('HeartbeatClient destroyed and resources cleaned up');
101+
}
92102
}

src/agent/protocol/grpc/clients/TraceReportClient.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,31 @@ export default class TraceReportClient implements Client {
3434
private readonly reporterClient: TraceSegmentReportServiceClient;
3535
private readonly buffer: Segment[] = [];
3636
private timeout?: NodeJS.Timeout;
37+
private segmentFinishedListener: (segment: Segment) => void;
3738

3839
constructor() {
3940
this.reporterClient = new TraceSegmentReportServiceClient(
4041
config.collectorAddress,
4142
config.secure ? grpc.credentials.createSsl() : grpc.credentials.createInsecure(),
4243
);
43-
emitter.on('segment-finished', (segment) => {
44+
45+
// Store listener reference for cleanup
46+
this.segmentFinishedListener = (segment: Segment) => {
47+
// Limit buffer size to prevent memory leak during network issues
48+
if (this.buffer.length >= config.maxBufferSize) {
49+
logger.warn(
50+
`Trace buffer reached maximum size (${config.maxBufferSize}). ` +
51+
`Discarding oldest segment to prevent memory leak. ` +
52+
`This may indicate network connectivity issues with the collector.`
53+
);
54+
this.buffer.shift(); // Remove oldest segment
55+
}
56+
4457
this.buffer.push(segment);
4558
this.timeout?.ref();
46-
});
59+
};
60+
61+
emitter.on('segment-finished', this.segmentFinishedListener);
4762
}
4863

4964
get isConnected(): boolean {
@@ -107,4 +122,22 @@ export default class TraceReportClient implements Client {
107122
this.reportFunction(resolve);
108123
});
109124
}
125+
126+
destroy(): void {
127+
// Clean up event listener to prevent memory leak
128+
if (this.segmentFinishedListener) {
129+
emitter.off('segment-finished', this.segmentFinishedListener);
130+
}
131+
132+
// Clear timeout
133+
if (this.timeout) {
134+
clearTimeout(this.timeout);
135+
this.timeout = undefined;
136+
}
137+
138+
// Clear buffer
139+
this.buffer.length = 0;
140+
141+
logger.info('TraceReportClient destroyed and resources cleaned up');
142+
}
110143
}

src/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,20 @@ class Agent {
7272
});
7373
});
7474
}
75+
76+
destroy(): void {
77+
if (this.protocol === null) {
78+
logger.warn('Trying to destroy() SkyWalking agent which is not started.');
79+
return;
80+
}
81+
82+
logger.info('Destroying SkyWalking agent and cleaning up resources');
83+
84+
// Clean up protocol resources
85+
this.protocol.destroy?.();
86+
this.protocol = null;
87+
this.started = false;
88+
}
7589
}
7690

7791
export default new Agent();

0 commit comments

Comments
 (0)