Web interface console messages seen duplicated

[justparking]

I noticed when two instances are running, the second one can duplicate the web interface messages.

Demonstrated by example:

\any_dir> java -jar \nodelhost-dev-2.2.1-rev493.jar
Nodel [Jython] v2.2.1-dev_r493 is running.

Press Enter to initiate a shutdown.

    (web interface available at http://172.19.240.1:8085)

    (web interface available at http://172.21.64.1:8085)

    (web interface available at http://172.25.224.1:8085)

    (web interface available at http://192.168.98.132:8085)

    (web interface available at http://172.30.48.1:8085)

    (web interface available at http://10.78.0.225:8085)

Then at the same time in a CLEAN folder

\clean_dir>java -jar \nodelhost-dev-2.2.1-rev493.jar -p 0
Nodel [Jython] v2.2.1-dev_r493 is running.

Press Enter to initiate a shutdown.

    (web interface available at http://172.19.240.1:61333)

    (web interface available at http://172.21.64.1:61333)

    (web interface available at http://172.25.224.1:61333)

    (web interface available at http://192.168.98.132:61333)

    (web interface available at http://172.30.48.1:61333)

    (web interface available at http://10.78.0.225:61333)

    (web interface available at http://172.19.240.1:61333)

    (web interface available at http://172.21.64.1:61333)

    (web interface available at http://172.25.224.1:61333)

    (web interface available at http://192.168.98.132:61333)

    (web interface available at http://172.30.48.1:61333)

    (web interface available at http://10.78.0.225:61333)

I believe this relates to recent merges which contained WebSocket and HTTP port combining.

[morimoriysmoon]

@justparking
The callback (for TopologyWatcher) below gets called even though Launch class failed to start NodelHostHTTPD because of already bound port.

So I think we need to unregister the callback when it fails to start NodelHostHTTPD.

[morimoriysmoon]

A new branch added on my public forked repository.
e6b946a

[justparking]

Hey @morimoriysmoon - I see in your update you added extra code for a new "removeHander___" methods.

That class has always had a "removeHandler___" method so I'm not sure why extra have been added?

Had you tried to use it?

On a side note: Looking at the original code a little closer I do notice that there is a minor bug related to the missing clean up of the _removedOnChangeHandlers list BUT the _onChangeHandlers IS maintained correctly so it shouldn't affect its functionality i.e. it should still clean up any TopologyWatcher.ChangeHandler callbacks.

Adding new handlers is done making a snapshot and copying the new ones.

    synchronized (_newOnChangeHandlers) {
        if (_newOnChangeHandlers.size() > 0) {
            newHandlers = new ArrayList<ChangeHandler>(_newOnChangeHandlers);

            // no longer new now...
            _newOnChangeHandlers.clear();
        }
    }

Dealing with the removed handlers should be done in the same way which it is except for clearing the list.

(it's all done this way to make it safe no matter the threading environment)

[morimoriysmoon]

I don't think the existing "removeOnChangeHandler()" can solve this issue because of 3.
(tested and confirmed there is still duplicated logging with using existing _removedOnChangeHandlers)

1. "addOnChangeHandler()" is called by the constructor of NodelHostHTTPD class.
2. "addOnChangeHandler()" will register (or add) given callback into _newOnChangeHandlers (not _onChangeHandlers).
3. On "monitorInterfaces()", any callbacks which are found from _removedOnChangeHandlers will be unregisterd from _onChangeHandlers. But in this case, nothing is found on _onChangeHandlers.

I think the change below also can fix this issue.

Please advise me.

[justparking]

Yes, I agree. That should fix the problem (assuming there's no possibility of an exception being thrown).