Skip to content

Commit

Permalink
append: fix possible deadlock
Browse files Browse the repository at this point in the history
This construct enters into a deadlock if
1) the writer is waiting because the queue is full
2) the reader is waiting on the writer to quit

The reason is that this status is not communicated through the queue
but with a separate variable.

This commit makes it so the status is communicated only through the
queue itself.  Special note has to be taken in the case of a timeout
in the reader (res == fd->Timeout) since normally this branch does not
look at the queue at all.

Because of this we needed to add a function the the channel that
actively checks whether the other side is still alive.
  • Loading branch information
sebsura committed Feb 12, 2024
1 parent 0abd660 commit 9b4023d
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
14 changes: 13 additions & 1 deletion core/src/lib/channel.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
BAREOS® - Backup Archiving REcovery Open Sourced
Copyright (C) 2023-2023 Bareos GmbH & Co. KG
Copyright (C) 2023-2024 Bareos GmbH & Co. KG
This program is Free Software; you can redistribute it and/or
modify it under the terms of version three of the GNU Affero General Public
Expand Down Expand Up @@ -211,6 +211,18 @@ template <typename T> class input {
return false;
}

void try_update_status()
{
if (did_close) { return; }

bool should_close = false;
if (auto result = shared->try_input_lock()) {
if (std::get_if<channel_closed>(&result.value())) { should_close = true; }
}

if (should_close) { close(); }
}

void close()
{
if (!did_close) {
Expand Down
20 changes: 11 additions & 9 deletions core/src/stored/append.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
BAREOS® - Backup Archiving REcovery Open Sourced
Copyright (C) 2000-2012 Free Software Foundation Europe e.V.
Copyright (C) 2016-2023 Bareos GmbH & Co. KG
Copyright (C) 2016-2024 Bareos GmbH & Co. KG
This program is Free Software; you can redistribute it and/or
modify it under the terms of version three of the GNU Affero General Public
Expand Down Expand Up @@ -158,7 +158,7 @@ class MessageHandler {

BareosSocket* close_and_get_sock()
{
end.store(true);
output.close();
receive_thread.join();
return fd;
}
Expand All @@ -174,9 +174,6 @@ class MessageHandler {
{
}

bool error_while_reading{false};
std::atomic<bool> end{false};

BareosSocket* fd;
channel::input<result_type> input;
channel::output<result_type> output;
Expand Down Expand Up @@ -216,17 +213,22 @@ class MessageHandler {
fd->msg = nullptr;

if (!input.emplace(std::move(result))) {
Dmsg1(20,
"Tried to put message into queue; but it did not succeed.\n");
if (input.closed()) {
Dmsg1(20, "Tried to put message into closed queue.\n");
} else {
Dmsg1(20,
"Tried to put message into queue; but it did not succeed.\n");
}
cont = false;
}
} else if (res == fd->Error) {
cont = false;
error_while_reading = true;
} else {
ASSERT(res == fd->Timeout);
input.try_update_status();
}
if (end.load()) { cont = false; }

if (input.closed()) { cont = false; }
}

input.close();
Expand Down

0 comments on commit 9b4023d

Please sign in to comment.