Calling UserManager::isRegisteredUser(JID) from I/O handler thread

In PubSubEngine::createNodeHelper UserManager::isRegisteredUser(JID) is called from I/O handler thread. If user is not local, it sends disco#info query. Reply is handled by the same thread, so it will block for 1 minute without receiving answer.

1 Like

I’ve created a PR on GitHub which essentially avoids this issue for the components.

Do you have a Jira Account? Seems like a good idea to allow you to create issues there :slight_smile: If so, can you share that with me, so I can elevate your privs?

Note it continues as soon as the remote server responds; the minute timeout only applies if the remote server doesn’t reply any sooner.

However, I agree there’s a benefit to checking for a component first!

Please read carefully. If isRegisteredUser is called from elsewhere, it’s fine. However in this case it is called from the packet handler. Remote server responds, but packet handler for the answer will not be executed until previous packet handler completes, which in turn waits for the answer to be processed. So it will complete only after 1 minute timeout.

I’ve created https://issues.igniterealtime.org/browse/OF-1435 to deal with the minutes delay when a disco#info is required. I’m looking in to it now to see how best to resolve it.

I’ve tested this with two Openfire servers having a user from one server create a PubSub node on another. It seems that the bug does not appear in this case as the node creation request and the disco#info response go over different S2S connections and thus are handled by different threads. So it really only affected the components and that is fixed by my PR. Still, care should be taken in the future not to call isRegisteredUser if response is going to be handled by the same thread. I’m sorry for the confusion.

For what it’s worth, I can reliably reproduce (on a subscribe, rather than a create node, but the same principle applies).

It seems it’s necessary to have a S2S session up before the request is sent - this seems to ensure it’s sent across the same, existing session rather than one of the ones created as a part of the S2S dialback hand shakes.