Is QUI DI1 the correct way to mark a client as offline?

Asked by Corbin Simpson

Howdy,

I have been crafting a small ADC hub recently, based entirely off of the ADC protocol specification. Recently, I have been testing with DC++-based clients, including DC++ itself.

A curious problem has shown up. I randomly generate SIDs with no reuse, as others do. Occasionally, the list of connected clients will not clear in DC++. This usually happens after a reconnection, but it can also happen after a client has quit. If a client quits and reconnects, they will receive a new SID. DC++ will then *discard* the new information being broadcast for that client, because it still believes the other SID is still valid and ignores the new SID.

The end result is that DC++ now sees a ghost user that no longer exists and that the hub doesn't know about, and also denies the existence of an actual user that is definitely connected to the hub.

I have obtained the DC++ source corresponding to DC++ 0.802, and have been studying dcpp/AdcHub.cpp. I believe that I understand the anatomy of this bug, even if I don't know the correct resolution.

At dcpp/AdcHub.cpp:110, putUser() is defined. This appears to be one of the two points, along with clearUsers(), where users may be marked as offline. An attempt appears to be made to remove clients from the client list, but the SID-CID pairing is retained, leading to a conflict when the same CID reconnects. dcpp/AdcHub.cpp:152 inside handle(INF, ...) suggests "buggy hub" and gives the somewhat-dreaded "X has same CID as Y, ignoring" message.

Now, the hub does appear to have one recourse. When a DC++ client attempts to use an invalid SID, the hub could guess that the client is using a ghost SID which no longer exists. The hub could then send a message which is targeted to trigger putUser(SID, false) which will remove the ghostly SID from the DC++ client. The only such point is in handle(QUI, ...) at dcpp/AdcHub.cpp:305, where any QUI message with DI1 will definitely erase the nonexistent SID from DC++'s memory.

This technique works! It has a couple flaws, though. First, this conflicts with the protocol's idea of how QUI works, which is that QUI should *already* notify the client definitively that the given SID is deallocated. In fact, I have tried sending normal QUIs when clients disconnect, and sadly QUI does not remove entries from the client list nor erase the SID-CID mapping for that hub. DI1 is supposed to be for when a client is poisonous and should not be talked to under any circumstances; this is clearly both incorrect and overkill.

Second, the hub cannot always know which CID the DC++ client wanted to reference. Hubs are apparently not required to remember past clients' SIDs, and they can't do it in all circumstances. This bug can be triggered by the hub restarting, so persisting SIDs would take a fair amount of extra effort. It's possible, but no other hubs appear to do it. As a consequence, it's not really possible to know exactly which client's INF needs to be resent to DC++ to tell it about the *real* client that it wanted to connect to.

I'd love to have a better resolution to this problem, since it happens constantly and I can't really know enough about client-side state to be able to fix it every time.

~ C.

Question information

Language:
English Edit question
Status:
Answered
For:
DC++ Edit question
Assignee:
No assignee Edit question
Last query:
Last reply:
Revision history for this message
poy (poy) said :
#1

hey,

this forum isn't often used by ADC developers; it is more meant for simple DC++ usage problems. if you have more questions of the same kind, i highly recommend the dcdev hub <adcs://hub.dcbase.org:16591> and the dcbase forum <http://www.dcbase.org/forums/>.

regarding your issue: yes, a QUI message (DI1 is not important - it's only to ensure no client continues transfers when your hub is down, which is generally good for the wealth of the network) is the way to tell clients about one client going offline.

you write: "I randomly generate SIDs with no reuse, as others do." > reusing an SID after the client that had it before has left, and its peers notified of it with a QUI message, should not be a problem.

"An attempt appears to be made to remove clients from the client list, but the SID-CID pairing is retained" > as far as i can tell, users.erase(i); in the AdcHub::putUser function is enough to remove that SID-CID mapping. the remainder (OnlineUser pointer) is only used by the UI and should not interfere with other clients with the same SID connecting later on.

"When a DC++ client attempts to use an invalid SID" > i'm not sure what you mean here (since hubs are in charge of assining SIDs, not clients) but if you are referring to a client that would send a message like MSG or CTM with a wrong target SID, my opinion is that such a client is buggy and should be disconnected. this situation should not happen with DC++ as long as hubs correctly send QUI messages for all the disconnected clients (unless there is a bug here, but it's unlikely given the age and use of DC++'s ADC implementation).

"This technique works!" > please don't rely on it; as you already know, it's a workaround for a problem that shouldn't be happening. i suggest investigating why QUI messages on disconnection aren't leading to a user being removed in DC++ in the first place.

"This bug can be triggered by the hub restarting" > i disagree. when a hub restarts, all socket connections it has been maintaining with clients should close. this means that clients will see the hub go offline and they will clear their list of users as a result. "persisting SIDs" have no meaning in such a context: when the client sees the hub back on and connects to it again, it's just like a new hub; the SIDs it will now receive from that new hub don't matter. what matters to clients are CIDs; this is what they use to keep track of users in their queues / message tabs / etc.

for examples of current ADC hub implementations, i suggest taking a look at ADCH++ <https://launchpad.net/adchpp> (its adchpp/ClientManager.cpp file contains the user management part, including sending QUI etc) and uhub <https://www.uhub.org/>.

to ease testing and to easily see which commands are received on the client side, try the DC++ dev plugin <http://builds.dcbase.org/?dir=plugins>.

Can you help with this problem?

Provide an answer of your own, or ask Corbin Simpson for more information if necessary.

To post a message you must log in.