Prevent shutdown of shared thread pool

[janvyb]

Right now shared thread pools can be shut down, which leads to many problems (as described in #760 and #2465).

This PR fixes it by overriding shutdown and shutdownNow methods on wrapper classes around thread pools returned from factory methods on ThreadPoolManager, preventing shutdown and logging errors to help find out what addon calls them. This should not cause any issues since shared pools are never shut down by framework.

Closes #760

[kaikreuzer]

The QueueingThreadPoolExecutor is in a public package and thus a part of the openHAB Core API. It might thus be used in other places than the ThreadPoolManager. Overwriting its methods and logging of warnings is therefore dangerous.

I'd see two options:

1. Duplicate the class into an internal package and only change the internal one.
2. Move the class to an internal package. This would hence be API breaking, but as far as I can tell, no official code in openHAB makes use of this class, so this might be acceptable.

@janvyb and @openhab/core-maintainers WDYT?

[janvyb]

The QueueingThreadPoolExecutor is in a public package and thus a part of the openHAB Core API. It might thus be used in other places than the ThreadPoolManager. Overwriting its methods and logging of warnings is therefore dangerous.

Yeah, I can see that it might be a problem. How about creating another wrapper around ExecutorService and ScheduledExecutorService which would pass all methods to it's delegate except for shutdown and shutdownNow (something like Collections.unmodifiableList, but for schedulers), and then wrapping schedulers returned from methods on ThreadPoolManager?

[kaikreuzer]

ExecutorService is an interface, not a class. So I guess you mean essentially that we could create a subclass of QueueingThreadPoolExecutor instead of duplicating it - yes, that's indeed much better than option 1 above.

[janvyb]

What about this? It should be safe as long as noone cast the returned ExecutorService or ScheduledExecutorService into something else (which they should never do anyway). I'm not super happy about the wrapper's name but could not come up with anything better.

[kaikreuzer]

Might work that way. But what was wrong about the idea to create an internal subclass of QueueingThreadPoolExecutor?

[janvyb]

Might work that way. But what was wrong about the idea to create an internal subclass of QueueingThreadPoolExecutor?

I'd also have to create subclass of WrappedScheduledExecutorService since it's also being inserted into that pools hashmap inside getScheduledPool method. This way I don't have to care about specific implementation of ExecutorService or ScheduledExecutorService that's being returned.

[kaikreuzer]

Alright, thank you!
Let's merge and see if it all works as expected :-)