-
Notifications
You must be signed in to change notification settings - Fork 390
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
Netty executor leaking outside zio-http #1896
Comments
Is this the cause of the
I've seen since upgrading? |
@soujiro32167 yes, or at least that's what we're seeing in Caliban's tests as well. |
Thanks for confirming @frekw I was going nuts! |
I was able to mitigate by changing the netty pool to a dedicated one: // TODO: remove this when https://github.com/zio/zio-http/issues/1896 is fixed
val driver = ZLayer.scoped {
val app = ZLayer.succeed(
new AtomicReference[(HttpApp[Any, Throwable], ZEnvironment[Any])]((Http.empty, ZEnvironment.empty)),
)
val ecb = ZLayer.succeed(new AtomicReference[Option[Server.ErrorCallback]](Option.empty))
val time = ZLayer.succeed(ServerTime.make(1000 millis))
val serverChannelFactory: ZLayer[ServerConfig, Nothing, ChannelFactory[ServerChannel]] =
ChannelFactories.Server.fromConfig
val eventLoopGroup = EventLoopGroups.fromConfig
val nettyRuntime = NettyRuntime.usingDedicatedThreadPool // the magic is here
val serverChannelInitializer =
ServerChannelInitializer.layer
val serverInboundHandler = ServerInboundHandler.layer
val serverLayers = app ++
serverChannelFactory ++
(
(
(time ++ app ++ ecb) ++
(eventLoopGroup >>> nettyRuntime) >>> serverInboundHandler
) >>> serverChannelInitializer
) ++
ecb ++
eventLoopGroup
make
.provideSome[ServerConfig & Scope](
serverLayers,
)
}
val randomPortServer =
ZLayer.fromZIO(
zio.Random.RandomLive.nextIntBetween(1024, 65535)
.map(p => ServerConfig.default.port(p))
) >>> driver >>> Server.base |
@ghostdogpr can you check if my fix above solves your problem? |
I tried but I can't test because we're using zio-http through Tapir and there are many breaking changes so they're not binary compatible. I'll need tapir to be upgraded as well in order to test. |
Ok then let's get back to this once we have a new release and a compatible tapir. Hopefully it's fixed! |
I'll work to get Tapir updated as soon as we have a new release of zio-http! |
(with that said, maybe we can cut a 0.0.4 soon to get the new |
@jdegoes ? :) |
@vigoo Yes, let's get your latest round of changes in and do 0.0.4! |
Ok, here we go: softwaremill/tapir#2702 Hopefully we can get a snapshot soon and verify if it fixes the issue 🤞 |
Unfortunately seems like this didn't fix it: ghostdogpr/caliban#1521 :/ |
Also, @soujiro32167:s workaround to use a dedicated threadpool doesn't really work since |
@vigoo do you want to re-open this or should we submit it as a new issue? |
Reopened, I will take another look! Thanks for trying out |
@frekw the object and the |
As far as I can tell they are now: https://github.com/zio/zio-http/blob/main/zio-http/src/main/scala/zio/http/netty/NettyRuntime.scala#L73 🤷 |
Nooooo |
I've managed to produce a minimal test that reproduces the behaviour, so at least now it should be easier to verify: https://github.com/zio/zio-http/pull/1978/files If you remove the |
Managed to reduce it slightly more so my previous comment is out of date. Just doing |
Will take a look at this. |
We have test failures with
RejectedExecutionException
in Caliban when upgrading to the latest version of zio-http (0.0.3).What happens is that we have some tests that start a zio-http server. I noticed that zio-http creates its own executor and calls
onExecutor
to override the default (seeusingSharedThreadPool
inNettyRuntime
). The problem is that when the test ends, this executor gets closed when the server'sZLayer
is terminated, but the zio runtime attempts at running more things on this executor afterwards (in our case, interrupting something inTestClock
), triggering theRejectedExecutionException
. It looks as if the executor leaks outside of its intended scope. Any idea about this?The text was updated successfully, but these errors were encountered: