client context. Prevent transport dtor under channel mutex.

m_channelMutex is locked recursively, so places like
disconnect(bool,bool) which purport to unlock for the
Transport detory actually don't as they are called
with m_channelMutex already locked.

Make a rather ugly hack of saving a ref to m_channel
at the various entry point.
This commit is contained in:
Michael Davidsaver
2018-04-04 19:25:04 -07:00
parent c9c8fb369a
commit 1e0523b7b9

View File

@@ -3305,12 +3305,16 @@ private:
virtual void destroy() OVERRIDE FINAL
{
// Hack. Prevent Transport from being dtor'd while m_channelMutex is held
Transport::shared_pointer old_transport;
{
Lock guard(m_channelMutex);
if (m_connectionState == DESTROYED)
return;
REFTRACE_DECREMENT(num_active);
old_transport = m_transport;
m_getfield.reset();
// stop searching...
@@ -3441,7 +3445,11 @@ public:
void disconnect() {
{
// Hack. Prevent Transport from being dtor'd while m_channelMutex is held
Transport::shared_pointer old_transport;
Lock guard(m_channelMutex);
old_transport = m_transport;
// if not destroyed...
if (m_connectionState == DESTROYED)
throw std::runtime_error("Channel destroyed.");
@@ -3462,12 +3470,14 @@ public:
*/
virtual void createChannelFailed() OVERRIDE FINAL
{
// Hack. Prevent Transport from being dtor'd while m_channelMutex is held
Transport::shared_pointer old_transport;
Lock guard(m_channelMutex);
// release transport if active
if (m_transport)
{
m_transport->release(getID());
m_transport.reset();
old_transport.swap(m_transport);
}
// ... and search again, with penalty
@@ -3607,9 +3617,12 @@ public:
}
virtual void searchResponse(const ServerGUID & guid, int8 minorRevision, osiSockAddr* serverAddress) OVERRIDE FINAL {
// Hack. Prevent Transport from being dtor'd while m_channelMutex is held
Transport::shared_pointer old_transport;
Lock guard(m_channelMutex);
Transport::shared_pointer transport = m_transport;
if (transport.get())
Transport::shared_pointer transport(m_transport);
if (transport)
{
// GUID check case: same server listening on different NIF
@@ -3626,7 +3639,7 @@ public:
// NOTE: this creates a new or acquires an existing transport (implies increases usage count)
transport = m_context->getTransport(internal_from_this(), serverAddress, minorRevision, m_priority);
if (!transport.get())
if (!transport)
{
createChannelFailed();
return;
@@ -3646,7 +3659,7 @@ public:
m_allowCreation = false;
// check existing transport
if (m_transport.get() && m_transport.get() != transport.get())
if (m_transport && m_transport.get() != transport.get())
{
disconnectPendingIO(false);
@@ -3659,7 +3672,10 @@ public:
return;
}
m_transport = transport;
// rotate: transport -> m_transport -> old_transport ->
old_transport.swap(m_transport);
m_transport.swap(transport);
m_transport->enqueueSendRequest(internal_from_this());
}
}