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

Provide the means in client builder to configure any service threads created as daemon threads #1136

Closed
jcferretti opened this issue May 26, 2023 · 1 comment

Comments

@jcferretti
Copy link
Contributor

jcferretti commented May 26, 2023

Is your feature request related to a problem? Please describe.
jetcd uses vertx and by default service threads created by vertx are not daemon threads. This means they will prevent the JVM from exiting if the main thread (and any other non-daemon threads) exit. We use jetcd in production and have frequenty run into the issue of uncaught exceptions in main, which we would normally expect to kill our processes, to leave the process around due to vertx threads not exiting:

"vert.x-eventloop-thread-0" #15 prio=5 os_prio=0 tid=0x00007f881976a800 nid=0x6921 runnable [0x00007f87bcfba000]
   java.lang.Thread.State: RUNNABLE
        at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
        at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
        at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
        at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
        - locked <0x00000006c1bf2fa8> (a io.deephaven.shadow.jetcd.io.netty.channel.nio.SelectedSelectionKeySet)
        - locked <0x00000006c1bf4028> (a java.util.Collections$UnmodifiableSet)
        - locked <0x00000006c1bf2f60> (a sun.nio.ch.EPollSelectorImpl)
        at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
        at io.deephaven.shadow.jetcd.io.netty.channel.nio.SelectedSelectionKeySetSelector.select(SelectedSelectionKeySetSelector.java:62)
        at io.deephaven.shadow.jetcd.io.netty.channel.nio.NioEventLoop.select(NioEventLoop.java:883)
        at io.deephaven.shadow.jetcd.io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:526)
        at io.deephaven.shadow.jetcd.io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
        at io.deephaven.shadow.jetcd.io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.deephaven.shadow.jetcd.io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.lang.Thread.run(Thread.java:750)

Describe the solution you'd like
The default behavior for vertx is for its service threads to be created as non-daemon. It is possible to configure vertx to use daemon threads by passing a VertxOptions object with the appropriate setup to the vertx factory method. Providing a way to configure service threads as daemon in the client builder, and propagating that value to the vertx factory method call in ClientConnectionManager would solve the issue.

Describe alternatives you've considered
Calling ClientConnectionManager.stop() in turn calls vertx.stop() on its vertex object, which would solve the issue if we were able to consistently call stop from all the relevant places. As our infrastructure stands, we have several places where jetcd clients are created from singleton objects and sadly there is no clean/easy way to close all of them when main exits. An alternative we considered was to install an uncaught exception handler in main's ThreadGroup, and execute code there to close all existing clients. That would require creating some global data structure for client registration and keeping that up to date from multiple sources. That is a lot of infrastructure compared to the idea proposed in this issue. Moreover, the default implementation of most network frameworks and libraries in java that need to crate service threads creates them as daemon, so vertx, and by extension jetcd, are outliers here.

Additional context
https://vertx.io/docs/apidocs/io/vertx/core/VertxOptions.html#setUseDaemonThread-java.lang.Boolean-
https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.UncaughtExceptionHandler.html
https://vertx.io/docs/vertx-core/java/#_causing_vert_x_to_exit

@jcferretti
Copy link
Contributor Author

After discussion during review it was preferred to not add a new configuration option, but just always setup vertx to create its service threads as daemon threads. Discussed in #1137, PR redone in #1146.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant