Skip to content

Commit

Permalink
[fold] minor cleanups in database stopping
Browse files Browse the repository at this point in the history
  • Loading branch information
seelabs committed Jul 18, 2022
1 parent 2939da2 commit d28da66
Showing 1 changed file with 18 additions and 19 deletions.
37 changes: 18 additions & 19 deletions src/ripple/nodestore/impl/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,31 +68,33 @@ Database::Database(

decltype(read_) read;

while (!isStopping())
while (1)
{
{
std::unique_lock<std::mutex> lock(readLock_);

if (isStopping())
break;

// We need to check whether we're stopping again to
// avoid a race condition that can stall this thread
// during shutdown.
if (read_.empty() && !isStopping())
if (read_.empty())
{
runningThreads_--;
readCondVar_.wait(lock);
runningThreads_++;
}

// If we are not stopping then extract multiple object
// at a time to minimize the overhead of acquiring the
// mutex.
if (!isStopping())
{
for (int cnt = 0;
!read_.empty() && cnt != requestBundle_;
++cnt)
read.insert(read_.extract(read_.begin()));
}
if (isStopping())
break;

// extract multiple object at a time to minimize the
// overhead of acquiring the mutex.
for (int cnt = 0;
!read_.empty() && cnt != requestBundle_;
++cnt)
read.insert(read_.extract(read_.begin()));
}

for (auto it = read.begin(); it != read.end(); ++it)
Expand Down Expand Up @@ -201,15 +203,12 @@ Database::asyncFetch(
std::uint32_t ledgerSeq,
std::function<void(std::shared_ptr<NodeObject> const&)>&& cb)
{
std::lock_guard lock(readLock_);

if (!isStopping())
{
std::lock_guard lock(readLock_);

if (!isStopping())
{
read_[hash].emplace_back(ledgerSeq, std::move(cb));
readCondVar_.notify_one();
}
read_[hash].emplace_back(ledgerSeq, std::move(cb));
readCondVar_.notify_one();
}
}

Expand Down

5 comments on commit d28da66

@greg7mdp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@greg7mdp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I had flagged this issue in May XRPLF#4152

@seelabs
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@greg7mdp Ugh. That really should not have happened.

@greg7mdp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be more vocal next time :-)

@seelabs
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't on you. The process broke down.

Please sign in to comment.