Skip to content

Commit 4588194

Browse files
authored
fix(emulator): clean up RTU transport event listener on initialization failure (#283)
Fixes #280 - RTU transport leaks event listener on initialization failure When start() fails due to openCallback error, the 'error' event listener was not removed from the server instance, causing memory leaks in applications that retry connections. Root cause: The openCallback error handler in start() rejected the Promise but didn't remove the 'error' listener attached after ServerSerial construction. This leaked 1 listener per failed start() attempt. Fix: Add EventEmitter.prototype.removeAllListeners call in openCallback error path to clean up the listener before rejecting, matching the pattern used in TCP transport (issue #274) and stop() method (issue #253). Also refactored to use local server variable instead of this.server in callbacks to avoid non-null assertion and match TCP transport pattern. Test coverage: Added regression test verifying no listeners remain after failed start() due to openCallback error. Test uses EventEmitter-based mock with prototype spy to properly verify cleanup. Pattern consistency: This completes the event listener cleanup pattern across transports. Matches TCP transport fix from #274 and stop() cleanup from #253. Closes #280 Related to #274, #253
1 parent bd6fad7 commit 4588194

2 files changed

Lines changed: 66 additions & 2 deletions

File tree

packages/emulator/src/transports/rtu.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,66 @@ describe('RtuTransport', () => {
191191
await expect(transport.start()).rejects.toThrow('Failed to open port')
192192
})
193193

194+
it('should clean up error listener when start() fails due to openCallback error', async () => {
195+
// Test for issue #280: RTU transport leaks 'error' event listener on initialization failure
196+
// When openCallback is called with an error, start() rejects but the 'error' listener
197+
// added on line 116 of rtu.ts is never removed, causing a memory leak
198+
const { EventEmitter } = await import('events')
199+
const { ServerSerial } = await import('modbus-serial')
200+
const startError = new Error('Failed to open port')
201+
let mockInstance: any
202+
;(ServerSerial as any).mockImplementationOnce((vector: any, options: any) => {
203+
capturedServiceVector = vector
204+
eventListeners = new Map()
205+
206+
// Simulate openCallback error (port can't be opened)
207+
if (options.openCallback) {
208+
setImmediate(() => options.openCallback(startError))
209+
}
210+
211+
// Create mock that inherits from EventEmitter to support EventEmitter.prototype.removeAllListeners.call()
212+
const localMockInstance = Object.create(EventEmitter.prototype)
213+
EventEmitter.call(localMockInstance)
214+
215+
// Add mock methods and properties
216+
localMockInstance.close = jest.fn((cb: (err: Error | null) => void) => cb(null))
217+
localMockInstance.socks = new Map()
218+
219+
// Wrap the on() method to track listeners in both EventEmitter and test Map
220+
const originalOn = localMockInstance.on.bind(localMockInstance)
221+
localMockInstance.on = jest.fn((event: string, listener: (...args: unknown[]) => void) => {
222+
// Track in test Map for assertions
223+
if (!eventListeners.has(event)) {
224+
eventListeners.set(event, new Set())
225+
}
226+
eventListeners.get(event)!.add(listener)
227+
// Register with real EventEmitter
228+
return originalOn(event, listener)
229+
})
230+
231+
mockInstance = localMockInstance
232+
return localMockInstance
233+
})
234+
235+
// Spy on EventEmitter.prototype.removeAllListeners to intercept calls via .call()
236+
const prototypeSpy = jest.spyOn(EventEmitter.prototype, 'removeAllListeners')
237+
238+
transport = new RtuTransport({ port: '/dev/ttyUSB0' })
239+
240+
// Start should fail due to openCallback error
241+
await expect(transport.start()).rejects.toThrow('Failed to open port')
242+
243+
// After the fix: removeAllListeners('error') should be called during failed start
244+
// Before the fix: no cleanup happens, 'error' listener remains on the server instance
245+
expect(prototypeSpy).toHaveBeenCalledWith('error')
246+
247+
// Verify that the 'error' listener was actually removed from EventEmitter
248+
expect(mockInstance.listenerCount('error')).toBe(0)
249+
250+
// Clean up spy
251+
prototypeSpy.mockRestore()
252+
})
253+
194254
it('should reject on stop error', async () => {
195255
const { ServerSerial } = await import('modbus-serial')
196256
const closeError = new Error('Failed to close port')

packages/emulator/src/transports/rtu.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,14 @@ export class RtuTransport extends BaseTransport {
9191

9292
// Create and start server
9393
return new Promise<void>((resolve, reject) => {
94-
this.server = new ServerSerial(
94+
const server = new ServerSerial(
9595
serviceVector,
9696
{
9797
...options,
9898
openCallback: (err: Error | null) => {
9999
if (err) {
100+
// Clean up error listener to prevent memory leak (issue #280)
101+
EventEmitter.prototype.removeAllListeners.call(server, 'error')
100102
reject(err)
101103
} else {
102104
this.started = true
@@ -110,8 +112,10 @@ export class RtuTransport extends BaseTransport {
110112
}
111113
)
112114

115+
this.server = server
116+
113117
// Handle errors
114-
this.server.on('error', (err) => {
118+
server.on('error', (err) => {
115119
// Log error but don't stop server
116120
console.error('RTU transport error:', err)
117121
})

0 commit comments

Comments
 (0)