Skip to content

Commit

Permalink
Fail read calls after exception.
Browse files Browse the repository at this point in the history
If a read call was performed after XrdFile threw an exception, it
is possible there are no active sources.  However, not all paths
check for the case of zero sources and may use-after-free the last
source.

This caused a deadlock at the Tier-0.
  • Loading branch information
bbockelm committed Nov 29, 2015
1 parent fd1b067 commit a012aaf
Showing 1 changed file with 34 additions and 1 deletion.
35 changes: 34 additions & 1 deletion Utilities/XrdAdaptor/src/XrdRequestManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,17 @@ std::shared_ptr<XrdCl::File>
RequestManager::getActiveFile()
{
std::lock_guard<std::recursive_mutex> sentry(m_source_mutex);
if (m_activeSources.empty())
{
edm::Exception ex(edm::errors::FileReadError);
ex << "XrdAdaptor::RequestManager::getActiveFile(name='" << m_name
<< "', flags=0x" << std::hex << m_flags
<< ", permissions=0" << std::oct << m_perms << std::dec
<< ") => Source used after fatal exception.";
ex.addContext("In XrdAdaptor::RequestManager::handle()");
addConnections(ex);
throw ex;
}
return m_activeSources[0]->getFileHandle();
}

Expand Down Expand Up @@ -461,6 +472,17 @@ RequestManager::pickSingleSource()
m_nextInitialSourceToggle = true;
}
}
else if (m_activeSources.empty())
{
edm::Exception ex(edm::errors::FileReadError);
ex << "XrdAdaptor::RequestManager::handle read(name='" << m_name
<< "', flags=0x" << std::hex << m_flags
<< ", permissions=0" << std::oct << m_perms << std::dec
<< ") => Source used after fatal exception.";
ex.addContext("In XrdAdaptor::RequestManager::handle()");
addConnections(ex);
throw ex;
}
else
{
source = m_activeSources[0];
Expand Down Expand Up @@ -569,14 +591,25 @@ XrdAdaptor::RequestManager::handle(std::shared_ptr<std::vector<IOPosBuffer> > io
edm::CPUTimer timer;
timer.start();

assert(m_activeSources.size());
if (m_activeSources.size() == 1)
{
std::shared_ptr<XrdAdaptor::ClientRequest> c_ptr(new XrdAdaptor::ClientRequest(*this, iolist));
checkSources(now, c_ptr->getSize());
m_activeSources[0]->handle(c_ptr);
return c_ptr->get_future();
}
// Make sure active
else if (m_activeSources.empty())
{
edm::Exception ex(edm::errors::FileReadError);
ex << "XrdAdaptor::RequestManager::handle readv(name='" << m_name
<< "', flags=0x" << std::hex << m_flags
<< ", permissions=0" << std::oct << m_perms << std::dec
<< ") => Source used after fatal exception.";
ex.addContext("In XrdAdaptor::RequestManager::handle()");
addConnections(ex);
throw ex;
}

assert(iolist.get());
std::shared_ptr<std::vector<IOPosBuffer> > req1(new std::vector<IOPosBuffer>);
Expand Down

0 comments on commit a012aaf

Please sign in to comment.