Allow vertx thread to be daemon

[Apache9]

Describe the feature

Allow vertx thread to be daemon

Use cases

https://github.com/eclipse-vertx/vert.x/blob/2439c849549b08a1e7ffee8f09eae886c9fe88f6/src/main/java/io/vertx/core/impl/VertxImpl.java#L1144

Here we force setting the daemon to false when creating vertx thread. The reason is to prevent JVM exit for embedded user as said in the comment. But actually, the default thread factory will create non daemon threads, if users set a special thread factory to create daemon threads, they must have a reason. It does not make sense to reset the thread to non daemon under this scenario.

Contribution

The change is very simple, just remove the thread.setDaemon(false); line. I can open a PR for this.

[vietj]

that's a good point, however a thread can be a daemon when the thread that constructed it is a daemon itself, e.g

  public static void main(String[] args) throws Exception {
    System.out.println("1 daemon=" + new Thread().isDaemon());
    Thread t1 = new Thread(() -> {
      System.out.println("2 daemon=" + new Thread().isDaemon());
    });
    t1.setDaemon(true);
    t1.start();
    System.in.read();
  }

we can provide a setting for configuring that though (e.g VertxOptions)

[Apache9]

OK, check the code of VertxThreadFactory, the default implementation is not like JDK's DefaultThreadFactory...

  default VertxThread newVertxThread(Runnable target, String name, boolean worker, long maxExecTime, TimeUnit maxExecTimeUnit) {
    return new VertxThread(target, name, worker, maxExecTime, maxExecTimeUnit);
  }

        public Thread newThread(Runnable r) {
            Thread t = new Thread(group, r,
                                  namePrefix + threadNumber.getAndIncrement(),
                                  0);
            if (t.isDaemon())
                t.setDaemon(false);  // <===== here it will force set the daemon to false
            if (t.getPriority() != Thread.NORM_PRIORITY)
                t.setPriority(Thread.NORM_PRIORITY);
            return t;
        }

So I think maybe could change the default implementation of VertxThreadFactory, to force set thread to non daemon. In this way we do not need to introduce a new config then since we have already support users to specify a customized thread factory.

WDYT?

Thanks.

[vietj]

I don't think so, that should be a configurable option, e.g DefaultThreadFactory make this configurable:

    public Thread newThread(Runnable r) {
        Thread t = newThread(FastThreadLocalRunnable.wrap(r), prefix + nextId.incrementAndGet());
        try {
            if (t.isDaemon() != daemon) {
                t.setDaemon(daemon);
            }

            if (t.getPriority() != priority) {
                t.setPriority(priority);
            }
        } catch (Exception ignored) {
            // Doesn't matter even if failed to set.
        }
        return t;
    }

[Apache9]

OK.

So here we plan to add a new option in VertxOptions? Then how could we pass this option to VertxThreadFactory? Add a new parameter called daemon for the newVertxThread method?

[vietj]

VertxThreadFactory has init method that is passed the builder from which options can be obtained.

So implementations can use it and keep it:

  VertxThreadFactory INSTANCE = new VertxThreadFactory() {
    boolean daemon;
    @Override
    public void init(VertxBuilder builder) {
      VertxThreadFactory.super.init(builder);
      daemon = // Read config
    }
  };

we would however remove the default method newVertxThread that does not take it in account (breaking change but well).

[Apache9]

Looking at the comment here

https://github.com/eclipse-vertx/vert.x/blob/2439c849549b08a1e7ffee8f09eae886c9fe88f6/src/main/java/io/vertx/core/spi/VertxServiceProvider.java#L26

/**
 * Entry point for loading Vert.x SPI implementations.
 *
 * @author <a href="mailto:[email protected]">Julien Viet</a>
 */
public interface VertxServiceProvider {

  /**
   * Let the provider initialize the Vert.x builder.
   *
   * @param builder the builder
   */
  void init(VertxBuilder builder);

}

It seems that the init method is used to initialize the builder, not for initializing the ServiceProvider?

[vietj]

I think it can be used for that purpose.

[Apache9]

I think it can be used for that purpose.

OK, then I think we should change the above javadoc. And we also need to document out which fields of the builder can be used in this method, as per initialization order, some fields of the builder will be null as we haven't initialized them yet.

If no other concerns, I could try to open a PR for this if you do not mind @vietj .

Thanks for the helping here!

[vietj]

Feel free to open a PR, I will review it.

[Apache9]

Thanks @vietj !