I was experimenting with Smack when I hit an issue when using the isConnected API. The way I have written my code, before calling connect, or sending a stanza, I check whether isConnected returns true. This method is also called from my reconnection logic.
The issue I observed was that a reconnection attempt was ignored because this method returned true. That’s when I realized that the connected member variable in AbstractXMPPConnection class can be set and accessed by potentially concurrent threads.
Since then, I changed the type of “connected” from a boolean to an AtomicBoolean. And I haven’t seen that issue anymore.
Before I submit a pull-request for this change, I just wanted to get thoughts on this.
The isConnected boolean is read by many threads but (ideally) only written by one. I deliberately did not make it volatile (or used an AtomicBoolean as you suggested, which is probably not necessary), because isConnected is an ephemeral check anyway. That is, it could be wrong the moment after you looked at the boolean.
So the conclusion is: Do not let something like a reconnection manager make decisions based on it (and yes I know, the reconnection manager in smack does that). Instead use soley an event based approach that is provided by ConnectionListener.