whizzosoftware / WZWave

A native Java library for interfacing with Z-Wave PC controllers
Eclipse Public License 1.0
33 stars 22 forks source link

Starts a thread that won't close #16

Closed anton-johansson closed 6 years ago

anton-johansson commented 6 years ago

If I pass in a serialPort that does not exist to the constructor of NettyZWaveController, it will create a thread that I can't stop, so I can't really close my application (without using something like System.exit(1), which I'd like to avoid).

controller.stop(); does nothing, because that method seems unimplemented.

I guess the #stop() method should be implemented? bootstrap.connect returns a ChannelFuture. It seems like you can do various things on this future, such as:

future.disconnect().await();
future.disconnect().sync();
future.close().await();
future.close().sync();
future.closeFuture().await();
future.closeFuture().sync();

Would this be something to look into?

whizzosoftware commented 6 years ago

Hello,

Thanks for reporting this. I took a quick look and it doesn't seem like the ChannelFuture returned by bootstrap.connect() helps here. The bootstrap.shutdown() method has been deprecated as well. It seems that shutting down the EventLoopGroup causes that thread to end gracefully. I've added that to the stop() method as well as a call to gracefully close the MapDb database. Let's see if this resolves the issue you are seeing.

anton-johansson commented 6 years ago

I tried that too, but it doesn't work for me at least.

It seems that the following call:

eventLoopGroup.shutdownGracefully().sync();

... blocks forever... Even if I would use #shutdownGracefully(1, 1, SECONDS);, it still blocks forever.

The only reason I noticed this is because I haven't gotten my Z-Wave gear yet. But I would like my application to handle these kinds of things in a good way. :)

If it's any help, the remaining thread seems to be SingleThreadEventExecutor.thread. I'll dig some more.

whizzosoftware commented 6 years ago

It could be that the eventLoopGroup is waiting for the channel to close. Have you tried a combination of:

channel.close();
eventLoopGroup.shutdownGracefully().sync();
anton-johansson commented 6 years ago

Nope, the thread seems to keep running...

The thread is running the loop in ThreadPerChannelEventLoop#run(). It seems when it checks isShuttingDown(), the state is always 2 (STARTED).bootstrap.group().shutdown();

Also, wouldn't it be nice if the shutdown was handled automatically? I mean, if the connection fails, I shouldn't have to call controller.close(); in my ZWaveControllerListener, right?

When I use the deprecated method bootstrap.group().shutdown(), the thread closes as expected. However, the deprecation points to shutdownGracefully, the one you used, which doesn't seem to work...

            bootstrap.connect(new RxtxDeviceAddress(serialPort)).addListener(new ChannelFutureListener() {
                @Override
                public void operationComplete(ChannelFuture future) throws Exception {
                    if (future.isSuccess()) {
                        sendDataFrame(new Version());
                        sendDataFrame(new MemoryGetId());
                        sendDataFrame(new InitData());
                    } else {
                        onZWaveConnectionFailure(future.cause());
                        bootstrap.group().shutdown(); // This is deprecated, but works.
                    }
                }
            });
anton-johansson commented 6 years ago

Okay, not sure why, but if I use bootstrap.group().shutdownGracefully().sync() it does not work. If I just use bootstrap.group().shutdownGracefully(); it does work... Feels strange to me, it returns a Future, I should be able to wait for it to complete?