narakai / mdnsjava Goto Github PK

What steps will reproduce the problem?
1. Have 3 network adapters (I have VMWare Installed which adds 2)
2. Run with '-B _myservice _tcp'  to find _myservice._tcp.local

Expected to see a ResolverListener receive the Message object, but it never 
does.

WireShark shows MDNS responses arriving at the correct port, the 
DatagramProcessor receives the packets, but the PacketRunner is never executed.

Using revision 62 on the trunk - 2.1.6 SNAPSHOT (maven). Windows 7 Pro.


The NetworkProcessor class has a ThreadPoolExecutor processorExecutor.  This 
will have two long-running threads for each network interface.  Executors.java 
has 'CORE_THREADS_NETWORK_EXECUTOR' set to 5.  This means that all 5 thread 
pools are filled by the NetworkProcessors for the three interfaces.  When a 
datagram Packet arrives, line 176 of DatagramProcessor.java attempts to run the 
PacketRunner to process the incoming mDNS response.  Since all 5 threads are 
currently running, no new thread starts up to process the incoming packet, and 
the response never arrives at higher levels of the application.

To temporarily fix, set the CORE_THREADS_NETWORK_EXECUTOR value equal to two 
times the number of network interfaces plus three - I set mine to 10.

To correct, perhaps have a different pool for PacketRunners than for the 
NetworkProcessor.

What steps will reproduce the problem?
1. Service shutdown and discovery

What is the expected output? What do you see instead?
Gets NullpointerException in:
public void setCacheMonitor(CacheMonitor monitor);
at
executor.shutdown();

What version of the product are you using? On what operating system?
2.1.5

Please provide any additional information below:

Get NullpointerException in executor.shutdown() when
having broken out executor.shutdown into method: shutdownExecutor(). This 
implies thread safety issue. Consider synchronizing calls to 
executor.shutdown() in some way.
See following code and attached png image and java source file:

    /**
     * Sets the CacheMonitor used to monitor cache data. Can be used to
     * implement Cache record refreshing.
     * 
     * @param monitor the CacheMonitor
     */
    public void setCacheMonitor(CacheMonitor monitor)
    {
        cacheMonitor = monitor;
        synchronized (lock) {
            if (cacheMonitor == null) {
                if (executor != null) {
                    shutdownExecutor(); //Broken out!
                    executor = null; //Probably set to null here(?)
                }
            } else {
                if (executor != null) {
                    shutdownExecutor(); //Broken out!
                }
                executor = Executors.newScheduledThreadPool(1);
                executor.scheduleAtFixedRate(new MonitorTask(this, this.cacheMonitor), 1, 1, TimeUnit.SECONDS);
            }
        }
    }

    private void shutdownExecutor() { //This method was broken out from above!
        new Exception("TRACE").printStackTrace();

        executor.shutdown();
    }

This modification lead to NullpointerException, though code is basically the 
same.
In between 'if(executor != null)'-check and executor.shutdown(), executor may be
set to null by another thread.

I have attached a source file with proposed changes. Please note though that 
they were not seriously thought through. Issues may still exist (deadlock?), 
though the new code seems to work. Changes are marked with //TODO ...

But it is sad, that original author do not migrate it to the github and don't 
upload to maven central repository, now exist many unofficial clones

https://github.com/search?utf8=%E2%9C%93&q=mdnsjava