Skip to content

Windows RDS crashes when many devices connect #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lo-simon merged 8 commits into from
Oct 21, 2025
Merged

Windows RDS crashes when many devices connect #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5c96bea
Prevent exception in the reverse_lock_guard destructor
lo-simon Jul 18, 2025
369a12d
Add a try-catch block around the model.write_lock() to handle lock ex…
lo-simon Jul 25, 2025
471ee55
Remove unneceesary try-catch block
lo-simon Jul 25, 2025
85ede28
Merge remote-tracking branch 'origin/master' into fix_heavy_registration
lo-simon Jul 28, 2025
73ba85d
Update Development/nmos/events_ws_api.cpp
lo-simon Jul 29, 2025
17488db
Update Development/nmos/registration_api.cpp
lo-simon Jul 29, 2025
99e6c1d
Merge branch 'master' into fix_heavy_registration
lo-simon Jul 29, 2025
a5ab729
Show a more descriptive boost lock error message on the HTTP response
lo-simon Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Development/nmos/api_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,12 @@ namespace nmos
slog::log<slog::severities::warning>(gate, SLOG_FLF) << "HTTP error: " << e.what() << " [" << e.error_code() << "]";
set_error_reply(res, status_codes::BadRequest, e);
}
// Boost lock error indicates cannot get lock, perhaps lock limit exceeded
catch (const boost::lock_error& e)
{
slog::log<slog::severities::error>(gate, SLOG_FLF) << "Boost lock error: " << e.what() << " Perhaps lock limit exceeded";
set_error_reply(res, status_codes::ServiceUnavailable, {}, U("Cannot get lock. Perhaps lock limit exceeded"));
}
// while a runtime_error (often) indicates an unimplemented feature
catch (const std::runtime_error& e)
{
Expand Down
45 changes: 30 additions & 15 deletions Development/nmos/events_ws_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,27 +504,42 @@ namespace nmos
// otherwise, there's actually work to do...

details::reverse_lock_guard<nmos::read_lock> unlock(lock);
// note, without atomic upgrade, another thread may preempt hence the need to recalculate expire_health/forget_health and least_health
auto upgrade = model.write_lock();
// note 1, without atomic upgrade, another thread may preempt hence the need to recalculate expire_health/forget_health and least_health
// note 2, the try-catch block is used here because the Windows version of the `boost::shared_mutex::lock` throws lock exceptions when
// it has reached the maximum number of 128 exclusive_waiting locks. As an alternative we could replace the initial
// model.read_lock() with model.write_lock(). Then, we could remove the reverse_lock_guard for switching from read_lock to write_lock.
for (;;)
{
try
{
auto upgrade = model.write_lock();

expire_health = health_now() - nmos::fields::events_expiry_interval(model.settings);
forget_health = expire_health - nmos::fields::events_expiry_interval(model.settings);
expire_health = health_now() - nmos::fields::events_expiry_interval(model.settings);
forget_health = expire_health - nmos::fields::events_expiry_interval(model.settings);

// forget all resources expired in the previous interval
forget_erased_resources(resources, forget_health);
// forget all resources expired in the previous interval
forget_erased_resources(resources, forget_health);

// expire all connections for which there hasn't been a heartbeat in the last expiry interval
const auto expired = erase_expired_resources(resources, expire_health, false, true);
// expire all connections for which there hasn't been a heartbeat in the last expiry interval
const auto expired = erase_expired_resources(resources, expire_health, false, true);

if (0 != expired)
{
slog::log<slog::severities::info>(gate, SLOG_FLF) << expired << " resources have expired";
if (0 != expired)
{
slog::log<slog::severities::info>(gate, SLOG_FLF) << expired << " resources have expired";

slog::log<slog::severities::too_much_info>(gate, SLOG_FLF) << "Notifying events websockets thread"; // and anyone else who cares...
model.notify();
}
slog::log<slog::severities::too_much_info>(gate, SLOG_FLF) << "Notifying events websockets thread"; // and anyone else who cares...
model.notify();
}

least_health = nmos::least_health(resources);
least_health = nmos::least_health(resources);
break;
}
catch (const std::exception& e)
{
slog::log<slog::severities::warning>(gate, SLOG_FLF) << "erase_expired_events_resources_thread error: " << e.what();
std::this_thread::yield();
}
}
}
}
}
9 changes: 1 addition & 8 deletions Development/nmos/query_ws_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,7 @@ namespace nmos
}

// send the messages without the lock on resources
try
{
details::reverse_lock_guard<nmos::write_lock> unlock{ lock };
}
catch (const std::exception& e)
{
slog::log<slog::severities::error>(gate, SLOG_FLF) << "Unlock error: " << e.what();
}
details::reverse_lock_guard<nmos::write_lock> unlock{ lock };

if (!outgoing_messages.empty()) slog::log<slog::severities::info>(gate, SLOG_FLF) << "Sending " << outgoing_messages.size() << " websocket messages";

Expand Down
45 changes: 30 additions & 15 deletions Development/nmos/registration_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,42 @@ namespace nmos
// otherwise, there's actually work to do...

details::reverse_lock_guard<nmos::read_lock> unlock(lock);
// note, without atomic upgrade, another thread may preempt hence the need to recalculate expire_health/forget_health and least_health
auto upgrade = model.write_lock();
// note 1, without atomic upgrade, another thread may preempt hence the need to recalculate expire_health/forget_health and least_health
// note 2, the try-catch block is used here because the Windows version of the `boost::shared_mutex::lock` throws lock exceptions when
// it has reached the maximum number of 128 exclusive_waiting locks. As an alternative we could replace the initial
// model.read_lock() with model.write_lock(). Then, we could remove the reverse_lock_guard for switching from read_lock to write_lock.
for (;;)
{
try
{
auto upgrade = model.write_lock();

expire_health = health_now() - nmos::fields::registration_expiry_interval(model.settings);
forget_health = expire_health - nmos::fields::registration_expiry_interval(model.settings);
expire_health = health_now() - nmos::fields::registration_expiry_interval(model.settings);
forget_health = expire_health - nmos::fields::registration_expiry_interval(model.settings);

// forget all resources expired in the previous interval
forget_erased_resources(resources, forget_health);
// forget all resources expired in the previous interval
forget_erased_resources(resources, forget_health);

// expire all nodes for which there hasn't been a heartbeat in the last expiry interval
const auto expired = erase_expired_resources(resources, expire_health, false);
// expire all nodes for which there hasn't been a heartbeat in the last expiry interval
const auto expired = erase_expired_resources(resources, expire_health, false);

if (0 != expired)
{
slog::log<slog::severities::info>(gate, SLOG_FLF) << expired << " resources have expired";
if (0 != expired)
{
slog::log<slog::severities::info>(gate, SLOG_FLF) << expired << " resources have expired";

slog::log<slog::severities::too_much_info>(gate, SLOG_FLF) << "Notifying query websockets thread"; // and anyone else who cares...
model.notify();
}
slog::log<slog::severities::too_much_info>(gate, SLOG_FLF) << "Notifying query websockets thread"; // and anyone else who cares...
model.notify();
}

least_health = nmos::least_health(resources);
least_health = nmos::least_health(resources);
break;
}
catch (const std::exception& e)
{
slog::log<slog::severities::warning>(gate, SLOG_FLF) << "erase_expired_resources_thread error: " << e.what();
std::this_thread::yield();
}
}
}
}

Expand Down
26 changes: 23 additions & 3 deletions Development/nmos/thread_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,26 @@ namespace nmos
public:
typedef BasicLockable mutex_type;
explicit reverse_lock_guard(mutex_type& m) : m(m) { m.unlock(); }
~reverse_lock_guard() { m.lock(); }
~reverse_lock_guard()
{
// note, the try-catch block is used here because the Windows version of the `boost::shared_mutex::lock` throws lock exceptions when
// it has reached the maximum number of 128 exclusive_waiting locks.

// ensure the lock is grabbed before return
for (;;)
{
try
{
m.lock();
break;
}
catch (...)
{
// ignore exception, and try again
std::this_thread::yield();
}
}
}
reverse_lock_guard(const reverse_lock_guard&) = delete;
reverse_lock_guard& operator=(const reverse_lock_guard&) = delete;
private:
Expand All @@ -29,8 +48,8 @@ namespace nmos
{
for (;;)
{
// Note: the try-catch block is here because Windows boost::condition_variable_any::wait can throw
// an exception once boost::shared_mutex has reached the maximum number of exclusive_waiting locks
// note, the try-catch block is used here because Windows boost::condition_variable_any::wait throws
// lock exception when boost::shared_mutex has reached the maximum number of 128 exclusive_waiting locks
try
{
if ((TimePoint::max)() == tp)
Expand All @@ -46,6 +65,7 @@ namespace nmos
catch (...)
{
// try the wait again
std::this_thread::yield();
}
}
}
Expand Down
Loading