-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NU-1609] fixed: Nu runtime memory leak #5898
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic!
@@ -16,6 +16,7 @@ | |||
* [#5843](https://github.com/TouK/nussknacker/pull/5843) Added new endpoint to provide statistics URL | |||
* [#5873](https://github.com/TouK/nussknacker/pull/5873) Handle list of the anonymous statistic URLs, and send them in the interval | |||
* [#5889](https://github.com/TouK/nussknacker/pull/5889) Decision Table component parameters validation improvement | |||
* [#5898](https://github.com/TouK/nussknacker/pull/5898) fixed Nu runtime memory leak |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will fix this later
// creation we clear the property (to not affect creating other /not existing currently/ IORuntimes). Note that | ||
// we change here the global state, so this is not a bullet proof solution. | ||
private def withDisableCatsTracingMode(create: => IORuntime): IORuntime = synchronized { | ||
val tracingModePropertyName = "cats.effect.tracing.mode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we declare this property at the beginning SynchronousExecutionContextAndIORuntime
:
private val TracingModePropertyName = "cats.effect.tracing.mode"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, IMO the current place of declaration is optimal
Describe your changes
SynchronousExecutionContextAndIORuntime
provides sync IORuntime. The runtime is rather dummy and there is no simple way to close it properly. We thought that there was no simple way to close it, but the closing is not required because it's a dummy. It figures out that it's not so true, because by default, during creation, MBean is registered. We don't need the monitoring functionality provided by the MBean, so we simply disable it before creating the sync IORuntime. Now, we consider it really a dummy one.Checklist before merge