Unihedro / JavaBot

Java based chat bot that runs in Java room of Stack Overflow Chat.
Apache License 2.0
15 stars 4 forks source link

Cleaning up `AbstractBot` #39

Closed Zomis closed 9 years ago

Zomis commented 9 years ago

There are currently two Executors in the AbstractBot class.

protected final ScheduledExecutorService   executor         = Executors.newSingleThreadScheduledExecutor();
protected final ExecutorService            processingThread = Executors.newSingleThreadExecutor();

One of them (executor) is shutdown inside the finalize method, which is a bad idea to rely on

The other one, processingThread is never shutdown at all, which makes a process running the JavaBot continue running because there is one or more threads that hasn't shutdown completely.

Vogel612 commented 9 years ago

Should be fixed in following commit: https://github.com/Vogel612/JavaBot/commit/043ab5521fb715f42baf8597c227e105adbb3ec7 already ;)

The branch needs some more simmering, but the next PR I open will include a fix for this

Zomis commented 9 years ago

There is a shutdown() method in DefaultBot, but it is never called from anywhere - and it only shuts down one of the executors (the same one that is shutdown in the finalize method). Perhaps consider adding a void shutdown() method to the ChatWorker interface? Either way, make sure that the executors are shutdown. (Don't use the finalize method!)

Unihedro commented 9 years ago

Thanks!