asynkron / protoactor-kotlin

Ultra-fast distributed cross-platform actor framework
http://proto.actor
Apache License 2.0
221 stars 25 forks source link

DeferredProcess causes a memory leak #38

Closed james-cobb closed 6 years ago

james-cobb commented 6 years ago

DeferredProcess adds itself to the process registry on creation. But this is entry is never removed. The Process Registry uses a map with strong references, so completed deferred processes are never garbage collected.

james-cobb commented 6 years ago

Possible solution is to remove registry entries in the await() method, in this branch:

https://github.com/crowdconnected/protoactor-kotlin/blob/49154c02a7cf518675a3b0d448bd79d62dfa4b00/proto-actor/src/main/kotlin/actor/proto/DeferredProcess.kt#L26

rogeralsing commented 6 years ago

This looks good! One option could be to remove the two remove calls and replace both with a single one inside a finally block

james-cobb commented 6 years ago

Yes, I had thought about using finally - but wan't sure on the behaviour if the exception is rethrown in the catch. Wanted to double check the finally is still called, and when (if the exception is caught further up, are any other catches executed before this finally, and which is preferred).

Needed a quick solution (currently in use under heavy load in a production environment!), will give some further thought to the best catch - finally structure, and make a PR.

rogeralsing commented 6 years ago

I'm currently stuck in NodeJS land due to work. The Kotlin version of ProtoActor could really use some love so I've sent an invite if you want to join in.

rogeralsing commented 6 years ago

@james-cobb are you still using this? I'm trying to get some time in to fix the current issue list

james-cobb commented 6 years ago

Yes, the current PR is still in use in production, has fixed the issue, and is stable.

Using a finally clause should work just as well, and seems neater. But I think the current solution actually makes the precise behaviour more explicit, at the expense of a duplicated line of code.

rogeralsing commented 6 years ago

👍 Thanks, pulled it all in. marking this as closed