typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
1.99k stars 511 forks source link

Remove IORuntime from allRuntimes on shutdown #4090

Closed RafalSierkiewicz closed 1 week ago

RafalSierkiewicz commented 1 week ago

The PR originated from question on discord https://discord.com/channels/632277896739946517/632278585700384799/1254741830042652772

Whenever the IORuntime is created through apply method it is being added to the allRuntimes collection. Unfortunately it seems that is not removed anywhere.

This PR tries to remove the created IORuntime from allRuntimes collection. I tried to keep the api as close as it was, but as the allRuntimes collection require runtime and the hashCode to be supplied to remove function I struggled a bit.

I am open for comments and suggestions. Let me know which branch I should target, not sure if change is backward and forward compatible

RafalSierkiewicz commented 1 week ago

I think it would make more sense to do this in the same local scope where we add to allRuntimes, both because it avoids this parameter juggle in the main class, but also because it keeps the logic a bit more localized. The forward references should be resolvable with some type ascription and strategic use of the lazy modifier.

Yes, I totally I agree. I was trying to achieve it, but somehow realized that might be hard 🙈 I changed it to use lazy as well as def and added test