Use of InvokeAndWait by AbstractMRTrafficController about jmri HOT 3 CLOSED

If I understand the problem correctly, invokeAndWait gives all listeners a chance to change the system state before the rest of the processing in handleOneIncomingReply() by invoking them on the UI thread in a Runnable.

However, I don't see what state can be changed by a listener that changes how handleOneIncomingReply completes its processing. I am also under the impression that the Runnable passed in invokeLater(Runnable) is guaranteed to be run in the order in which Runnables were queued by invokeLater(Runnable). So if listeners notified by the Runnable can't change how handleOneIncomingReply processes its data, and are guaranteed to be notified in packet order, what does the invokeAndWait buy us that invokeLater does not?

I think it buys us the ability to ensure all state is changed before the next reply is read, but don't know if that's a good thing or not.

from jmri.

I'll use C/MRI as an example. There are three threads running: line transmit (xmt for short), line receive (rcv) and Swing/AWT (AWT).

When the end of an incoming message is reached, the rcv thread drops through loadChars at AbstractTrafficController (ATC) line 852, notifies everybody of the response which processes inputs which can in turn generate output requests (see below). It then looks at the state, and in normal operation does a notify() of the xmt thread.

For C/MRI, the notify()ed xmt thread then runs the pollMessage() routine that goes through checking if nodes need to be initialized, if nodes have output that should be sent, and finally if any nodes need to be polled. It sends the 1st thing that scan finds.

invokeAndWait ensures that any output that's been generated by a poll response will go out right away. InvokeLater ensures that it won't. (The AWT/Swing events happen in the same order, because invokeLater and invokeAndWait invoke at the same point in the queue; they only differ in whether the posting thread waits)

Is that a problem? I don't know. I'm pretty sure that it wasn't thought through when that code was originally written. I don't think the "synchronized" attributes were checked with that in mind. Maybe it's fine. But I don't know that it is.

from jmri.

@pabender @bobjacobsen Thanks for the explanations here and in #422.

I'm thinking this problem can be safely ignored unless someone thinks they get a performance boost out of testing if InvokeLater() works for a specific system. I'm thinking that without physical access to a real system, changes will be hard to test.

from jmri.