Pluggable Discovery entry to START_SYNC state may miss hardware changes

[PaulStoffregen]

Having recently implemented Pluggable Discovery, how to properly begin START_SYNC mode was one of the places I applied some intuition and guesswork. But I'm a little worried it still may not be fully correct, and may not be possible with the current Pluggable Discovery specification.

The main question in my mind is regarding a possible (though unlikely) race between the protocol commands and hardware changes. Following the Pluggable Discovery spec, a client like arduino-cli would need to use the LIST command first to learn of all the currently connected devices. Then it would issue STOP to return to the idle state, and then START_SYNC to begin tracking changes. But what happens if hardware changes after the last LIST command, but before START_SYNC begins?

As a proactive and perhaps paranoid (maybe I worry about this stuff too much) measure, Teensy's implementation of START_SYNC transmits a set of eventType add messages for all currently known devices when entering START_SYNC mode. I believe elsewhere the spec says redundant "add" events are allowed and are to be interpreted as updating existing info. But maybe even this is not good enough? What happens if hardware is removed during that time between the last LIST and entering START_SYNC mode? Would arduino-cli forever believe that hardware is still present, because it was connected at the time of the last LIST, and a "remove" eventType message is never transmitted because it's already gone by the time START_SYNC is supposed to transmit real-time changes.

Maybe we should rethink the "start_sync" eventType message? Currently it just replies with OK. Perhaps it should reply with a list of all currently connected devices, similar to the LIST response? If so, the Pluggle Discovery spec should require that the transition from the list conveyed in the "start_sync" eventType message to be real-time reporting of changes be atomic with respect to hardware changes.

[matthijskooijman]

As a proactive and perhaps paranoid (maybe I worry about this stuff too much) measure, Teensy's implementation of START_SYNC transmits a set of eventType add messages for all currently known devices when entering START_SYNC mode. I believe elsewhere the spec says redundant "add" events are allowed and are to be interpreted as updating existing info.

AFAIU this is indeed the intention to prevent this race condition, except that the add messages are not just allowed, but required. The spec says:

After calling START_SYNC an initial burst of add events must be generated in sequence to report all the ports available at the moment of the start.

As for ports being removed, I read this as that the tool must generate add events for all ports that are present at the START_SYNC time, and arduino-cli should clear its list of available ports when emitting a START_SYNC, only adding ports that are emited after the START_SYNC (so any ports that have been removed between LIST and START_SYNC are gone, and in fact, there is probably no reason to call LIST in the first place).

See also previous discussion of exactly this race condition, Point 3 in arduino/tooling-rfcs#2 (comment) and arduino/tooling-rfcs#2 (comment)

Maybe we should rethink the "start_sync" eventType message? Currently it just replies with OK. Perhaps it should reply with a list of all currently connected devices, similar to the LIST response? If so, the Pluggle Discovery spec should require that the transition from the list conveyed in the "start_sync" eventType message to be real-time reporting of changes be atomic with respect to hardware changes.

I made exactly this suggestion in the comment above, which @cmaglie rejected saying:

The goal of the discovery is to list the available ports to allow the user to select one and upload to a board (we use it to populate the Tools->Port menu basically), so it doesn't matter if the board has been just plugged in or it was already present. Anyway, there should be no problems for GUIs (we are talking about a handful of ports not thousands!) IMHO it's not worth adding this extra complexity.

[PaulStoffregen]

Ok. Sounds like no issue here. I'm probably just overthinking things.