Skip to content
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

Merged
merged 5 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* [#5853](https://github.com/TouK/nussknacker/pull/5853) Add processing mode information and filtering to the components table
* [#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
* [#5898](https://github.com/TouK/nussknacker/pull/5898) fixed Nu runtime memory leak
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Contributor Author

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


1.14.0 (21 Mar 2024)
-------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ object SynchronousExecutionContextAndIORuntime extends LazyLogging {
})

private def createSyncIORuntime(): IORuntime = {
IORuntime(
compute = syncEc,
blocking = syncEc,
scheduler = DummyScheduler,
shutdown = () => {},
config = IORuntimeConfig()
)
withDisableCatsTracingMode {
IORuntime(
compute = syncEc,
blocking = syncEc,
scheduler = DummyScheduler,
shutdown = () => {},
config = IORuntimeConfig()
)
}
}

// note: we provide a dummy implementation of scheduler, because IORuntime requires some. We don't want to use real
Expand All @@ -49,4 +51,22 @@ object SynchronousExecutionContextAndIORuntime extends LazyLogging {
override def monotonicNanos(): Long = System.nanoTime()
}

// note: even if the provided by us IORuntime is dummy, it registers MBean for a sake of monitoring fibers. We don't
// need it, so we just disable it. There is no other way to do it by setting system property. After IORuntime
// 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"
Copy link
Contributor

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"

?

Copy link
Contributor Author

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

val originalTracingMode = Option(System.getProperty(tracingModePropertyName))
System.setProperty(tracingModePropertyName, "NONE")
try {
create
} finally {
originalTracingMode match {
case Some(mode) => System.setProperty(tracingModePropertyName, mode)
case None => System.clearProperty(tracingModePropertyName)
}
}
}

}
Loading