search

failsafe-lib / failsafe

Fault tolerance and resilience patterns for the JVM
https://failsafe.dev
Apache License 2.0
4.16k stars 295 forks source link

FailsafeFuture: clear execution after completion #362

Closed stevenschlansker closed 1 year ago

stevenschlansker commented 1 year ago

Fixes #361

jhalterman commented 1 year ago

Thanks for filing this issue and PR!

Since the reference chain you pointed out is FailsafeFuture -> AsyncExecutionImpl, I'm thinking it would be better to set newestExecution and cancelFunctions to null at the end of the FailsafeFuture.cancel and completeResult methods, since they may hold references to the execution. That doesn't address sync executions, which your PR does, but it doesn't seem like these would be a problem? Are you able to try that and confirm it resolves the problem on your side, and if so, update the PR?

stevenschlansker commented 1 year ago

Thanks @jhalterman , I made the requested change and I agree this should fix our problem. We don't care about sync executions.

jhalterman commented 1 year ago

Thanks @stevenschlansker !

jhalterman commented 1 year ago

This was released in Failsafe 3.3.1.