Invoking destroy() does not always terminate threads which prevents JVM from exiting.

[Andy Schneider]

    final class QueueConsumer implements Runnable {

        /**
         * Service the monitor service's queue.
         *
         * @see java.lang.Thread#run()
         */
        @Override
        public void run() {
            while (isActive()) {
                try {
                    InotifyEvent e = queue.take();

MonitorService.QueueConsumer.run() uses a blocking take() to remove an event from the queue. If there are no file system events and destroy() is called then take will block forever. This means that the while loop won't check isActive() and the thread won't terminate. In turn this means the JVM won't exit without calling System.exit().

        @Override
        public void run() {
            while (isActive()) {
                try {
                    InotifyEvent e = queue.poll(1, TimeUnit.SECONDS);

                    if (e == null) continue;

is the droid we're probably looking for.

[Andy Schneider (aschneider-getcollc)]

In the same function I also see null pointer exceptions on exit from the following code (line marked *** )

                    if (e.isIgnored()) {
                        if (path == null) path = watchPathMap.get(wd);
                        watchPathMap.remove(wd);
*** pathWatchMap.remove(path);
                        watchListenerMap.remove(wd);
                    }

In the above case I'd de-registered all listeners and watches. I fixed this this (and the issue described above) with this patch:

714c710,713
< InotifyEvent e = queue.take();
---
> InotifyEvent e = queue.poll(1, TimeUnit.SECONDS);
>
> if (e == null) continue;
>
744c743
< pathWatchMap.remove(path);
---
> if (path != null) pathWatchMap.remove(path);

BTW: Nice library.

[Nick Bargnesi (nbargnesi)]

Switching to a poll() wouldn't be the best mechanism here.
You'd be introducing a latency (your poll rate) and wasting CPU cycles if nothing is present on the queue.

Overriding destroy() and interrupting the consumer thread allows isActive() to get called.

Both producer/consumer thread should have been daemonized as well, to let the JVM gracefully exit when all user threads are done running.

The attached patch contains a potential fix for the null pointer potential you mention as well.

[Andy Schneider (aschneider-getcollc)]

xlnt. thnx.