Skip to content

Commit 7fcb352

Browse files
authored
Merge pull request #87 from bbockelm/crash_fixups
Fix deadlocks and crashes
2 parents ad3bec3 + 62e04f9 commit 7fcb352

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

src/S3File.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ S3File::DownloadBypass(off_t offset, size_t size, char *buffer) {
547547
return std::make_tuple(-1, 0, true);
548548
}
549549

550+
S3File::S3Cache::~S3Cache() {
551+
std::unique_lock lk(m_mutex);
552+
m_cv.wait(lk, [&] { return !m_a.m_inprogress && !m_b.m_inprogress; });
553+
}
554+
550555
bool S3File::S3Cache::CouldUseAligned(off_t req, off_t cache) {
551556
if (req < 0 || cache < 0) {
552557
return false;
@@ -974,14 +979,24 @@ void S3File::S3Cache::Entry::Download(S3File &file) {
974979
if (m_off + static_cast<off_t>(request_size) > file.content_length) {
975980
request_size = file.content_length - m_off;
976981
}
977-
if (!m_request->SendRequest(m_off, m_cache_entry_size)) {
982+
// This function is always called with m_mutex held; however,
983+
// SendRequest can block if the threads are all busy; the threads
984+
// will need to grab the lock to notify of completion. So, we
985+
// must release the lock here before calling a blocking function --
986+
// otherwise deadlock may occur.
987+
auto off = m_off;
988+
m_parent.m_mutex.unlock();
989+
if (!m_request->SendRequest(off, m_cache_entry_size)) {
990+
m_parent.m_mutex.lock();
978991
std::stringstream ss;
979992
ss << "Failed to send GetObject command: "
980993
<< m_request->getResponseCode() << "'"
981994
<< m_request->getResultString() << "'";
982995
file.m_log.Log(LogMask::Warning, "S3File::Read", ss.str().c_str());
983996
m_failed = true;
984997
m_request.reset();
998+
} else {
999+
m_parent.m_mutex.lock();
9851000
}
9861001
}
9871002

src/S3File.hh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,10 @@ class S3File : public XrdOssDF {
291291

292292
// Trigger a blocking read from a given file
293293
ssize_t Read(S3File &file, char *buffer, off_t offset, size_t size);
294+
295+
// Shutdown the cache; ensure all reads are completed before
296+
// deleting the objects.
297+
~S3Cache();
294298
};
295299
S3Cache m_cache;
296300
};

test/s3-setup.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ echo "Hello, World" > "$RUNDIR/hello_world.txt"
204204
IDX=0
205205
COUNT=25
206206
while [ $IDX -ne $COUNT ]; do
207-
if ! dd if=/dev/urandom "of=$RUNDIR/test_file" bs=1024 count=1024 2> /dev/null; then
207+
if ! dd if=/dev/urandom "of=$RUNDIR/test_file" bs=1024 count=3096 2> /dev/null; then
208208
echo "Failed to create random file to upload"
209209
exit 1
210210
fi

0 commit comments

Comments
 (0)