Hub invocation callbacks are not cleaned up in JS client if no response is received causing a memory leak #2270

Closed
DamianEdwards opened this Issue Jul 8, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@DamianEdwards
Member

DamianEdwards commented Jul 8, 2013

There's a memory leak when the hub proxy never receives a valid response from the server as the callback isn't cleaned up from the global callbacks map. Suggested changes:

  • Hub invocation callbacks should be stored on the connection instance metadata member (connection._). Note it needs to be separate to the existing callback map on there used to store hub proxy client callbacks
  • For the transports that use Ajax sends, the fail delegate should clear the callback delegate
  • For WebSockets, the callback map for that connection should be completely cleared on the reconnect event
  • For all transports, the callback map for that connection should be completely cleared on the disconnect event
  • Before a callback is deleted its associated deferred promise should be rejected with a suitable error

@ghost ghost assigned NTaylorMullen Jul 8, 2013

@ghost ghost assigned DamianEdwards Jul 10, 2013

DamianEdwards added a commit that referenced this issue Jul 11, 2013

Store hub invocation callbacks on connection instance
- Clear all callbacks on disconnect
- #2270

DamianEdwards added a commit that referenced this issue Jul 12, 2013

DamianEdwards added a commit that referenced this issue Jul 12, 2013

Ensure promises are rejected for failed hub invocations
- Hub invocation callbacks now cleared on websocket reconnect
- Hub invocation callbacks now cleared on failed ajax send
- All promises for failed hub invocations are not rejected
- Updated tests
- #2270

@DamianEdwards DamianEdwards referenced this issue Jul 13, 2013

Merged

2270 #2289

DamianEdwards added a commit that referenced this issue Jul 13, 2013

DamianEdwards added a commit that referenced this issue Jul 13, 2013

@ghost ghost assigned gustavo-armenta Jul 15, 2013

@gustavo-armenta

This comment has been minimized.

Show comment Hide comment
@gustavo-armenta

gustavo-armenta Jul 16, 2013

Contributor

Use branch test2270
Deploy server in one machine and start browser on another machine
Start IE10, navigate to http://server/Hubs/HubConnectionAPI/ and it should connect with signalr websocket, its using 30MB of memory. Click on button "LongRunning", it creates 50,000 callbacks and memory grows to 650MB.

Test Variations

  1. Stopped connection and memory usage lowered to 90MB
  2. Navigated to same webpage and memory usage lowered to 60MB
  3. Navigated to root webpage and memory usage lowered to 60MB
  4. Killed webserver, signalr connection tried to reconnect, reached disconnected state, and memory usage did not decrease
  5. Recycled app pool, signalr connection reconnected, and memory usage did not decrease

Expected
All test varations should see reduced memory usage

Actual
Scenarios 4 and 5 most of the time do not reduce memory usage

Contributor

gustavo-armenta commented Jul 16, 2013

Use branch test2270
Deploy server in one machine and start browser on another machine
Start IE10, navigate to http://server/Hubs/HubConnectionAPI/ and it should connect with signalr websocket, its using 30MB of memory. Click on button "LongRunning", it creates 50,000 callbacks and memory grows to 650MB.

Test Variations

  1. Stopped connection and memory usage lowered to 90MB
  2. Navigated to same webpage and memory usage lowered to 60MB
  3. Navigated to root webpage and memory usage lowered to 60MB
  4. Killed webserver, signalr connection tried to reconnect, reached disconnected state, and memory usage did not decrease
  5. Recycled app pool, signalr connection reconnected, and memory usage did not decrease

Expected
All test varations should see reduced memory usage

Actual
Scenarios 4 and 5 most of the time do not reduce memory usage

@ghost ghost assigned DamianEdwards Jul 16, 2013

@gustavo-armenta

This comment has been minimized.

Show comment Hide comment
@gustavo-armenta

gustavo-armenta Jul 17, 2013

Contributor

When I recycle web app pool, IE11 signalr reconnects and memory is still very high sometimes. Then, I take a memory snapshot and the memory goes down. It seems JS garbage collector is not kicked on reconnecting or even if I wait a couple of mins on the browser.
I am not sure if we can do something about it.

Contributor

gustavo-armenta commented Jul 17, 2013

When I recycle web app pool, IE11 signalr reconnects and memory is still very high sometimes. Then, I take a memory snapshot and the memory goes down. It seems JS garbage collector is not kicked on reconnecting or even if I wait a couple of mins on the browser.
I am not sure if we can do something about it.

@DamianEdwards

This comment has been minimized.

Show comment Hide comment
@DamianEdwards

DamianEdwards Jul 18, 2013

Member

Can you try forcing a GC in the browser after reconnect to see if it's reclaimed then? See http://stackoverflow.com/questions/2760285/how-can-i-force-javascript-garbage-collection-in-ie-ie-is-acting-very-slow-afte

Member

DamianEdwards commented Jul 18, 2013

Can you try forcing a GC in the browser after reconnect to see if it's reclaimed then? See http://stackoverflow.com/questions/2760285/how-can-i-force-javascript-garbage-collection-in-ie-ie-is-acting-very-slow-afte

DamianEdwards added a commit that referenced this issue Jul 18, 2013

Ensure clearing callbacks is re-entrant
- This also appears to more aggressively reclaim memory on reconnect for websockets
- Added a test for large callback list
- Scripts in all projects updated
- #2270

DamianEdwards added a commit that referenced this issue Jul 19, 2013

Prepare #2270 for merge to dev branch
Merged in fix for #2270 from release1.1

Conflicts:
	src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.core.js
	src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.hubs.js
	src/Microsoft.AspNet.SignalR.Client.JS/jquery.signalR.transports.common.js
	tests/Microsoft.AspNet.SignalR.Client.JS.Tests/Tests/FunctionalTests/Hubs/HubProxyFacts.js

@ghost ghost assigned gustavo-armenta Jul 20, 2013

@gustavo-armenta

This comment has been minimized.

Show comment Hide comment
@gustavo-armenta

gustavo-armenta Jul 23, 2013

Contributor

with fix for re-entrant, I see the memory is cleaned every time there is a signalr reconnection

Contributor

gustavo-armenta commented Jul 23, 2013

with fix for re-entrant, I see the memory is cleaned every time there is a signalr reconnection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment