Skip to content

Commit

Permalink
Fixes a static initialization bug under linux. (#571)
Browse files Browse the repository at this point in the history
The osGlobalAtomic variable was accessed from the non-default threads while the
static initializers were running. This caused the mutex to be lost sometimes.
After this fix a static Atomic variable does not have a constructor call, but is
linker-initialized from the .data section.

This bug caused the hub application to get sometimes "livelocked" on linux, with the
packets printed to the screen but not being sent to any connected devices actually.
  • Loading branch information
balazsracz authored Sep 11, 2021
1 parent bc8162e commit 040bce0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
6 changes: 4 additions & 2 deletions src/os/OSImpl.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ static Atomic osGlobalAtomic;

extern "C" {

void os_atomic_lock() {
void os_atomic_lock()
{
osGlobalAtomic.lock();
}

void os_atomic_unlock() {
void os_atomic_unlock()
{
osGlobalAtomic.unlock();
}

Expand Down
19 changes: 17 additions & 2 deletions src/utils/Atomic.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,24 @@ private:
/// Usage: Declare Atomic as a private base class, add a class member
/// variable or a global variable of type Atomic. Then use AtomicHolder to
/// protect the critical sections.
class Atomic : public OSMutex {
class Atomic
{
public:
Atomic() : OSMutex(true) {}
void lock()
{
os_mutex_lock(&mu_);
}
void unlock()
{
os_mutex_unlock(&mu_);
}
private:
/// Mutex that protects.
///
/// NOTE: it is important that this be trivially initialized and the Atomic
/// class have no (nontrivial) constructor. This is the only way to avoid
/// race conditions and initialization order problems during startup.
os_mutex_t mu_ = OS_RECURSIVE_MUTEX_INITIALIZER;
};

#endif
Expand Down

0 comments on commit 040bce0

Please sign in to comment.