[XrdCl] Avoid race condition in AsyncSocketHander on use of reader/writer objects after link is re-enabled

[smithdh]

Hi,

This patch is intended to fix a crash seen on EOS MGMs. The crash is consistent with the race between reset() and use of the unique_ptr rspreader and reqwriter. A trace obtained indicated a reset was happening in AsyncSocketHandler::OnReadTimeout() while the unique_ptr was being sued concurrently in AsyncMsgWriter::Write().

The race is understood to be started in AsyncSocketHandler::OnReadTimeout(), triggered by the pStream->OnReadTimeout call (XrdClAsyncSocketHandler.cc:698). On this timeout condition the XrdClStream may first Close() the AsyncSocketHandler. Close() calls pPoller->RemoveSocket(), which eventually calls XrdSys::IOEvents::Channel::Delete(), which will remove the socket from the IOEvents poller with XrdSys::IOEvents::Poller::Detach().

The above calls are done under the OnReadTimeout(), inside a callback from within the poller's event loop. The Detach correctly handles the situation where it is called from the poller's event loop, without blocking.

A later call by XrdClStream (in the same thread) reenables the link with EnableLink() in Stream::OnError(), which will call AsyncSocketHandler::Connect(). This will then re-add the socket to a poller by calling PollerBuiltIn::AddSocket(). AddSocket will assign the socket's XrdSys::IOEvents::Channel to a XrdSys::IOEvents::Poller from its poller pool. SInce EOS uses XRD_PARALLELEVTLOOP=4 the new poller may not be the same as the original (assuming the closed stream was the only one registered for the channelId). The AddSocket() is still being called from within the original poller's event loop, but if it is re-added to a different poller the AsyncSocketHandler event callbacks can now be called concurrently with the ongoing execution of the original

[amadio]

@smithdh I'd consider adding the explanation above to the commit message, it's very good. Cheers,

[simonmichal]

@smithdh : thank you very much for the PR, it's superb :-) !!!