Skip to content

Fixed a data race in HeterCallbackList: create the callback lists in the constructor - #93

Open
Haititi wants to merge 1 commit into
wqking:masterfrom
Haititi:fix/hetercallbacklist-eager-lists
Open

Haititi wants to merge 1 commit into
wqking:masterfrom
Haititi:fix/hetercallbacklist-eager-lists

Conversation

@Haititi

@Haititi Haititi commented Oct 6, 2026

Copy link
Copy Markdown

The callback list of each prototype was created on the first use with double checked locking, but the first check read the std::shared_ptr without the lock while another thread could be assigning it under the lock. std::shared_ptr is not atomic, so that's a data race (ThreadSanitizer reports it when several threads use a fresh HeterCallbackList or HeterEventDispatcher at the same time).

Now all the lists are created in the constructor (the count of prototypes is a compile time constant, usually a few), so looking up a list needs no lock at all. A moved-from object gets new empty lists so it's still usable, as before. callbackListListMutex is kept for the operations that modify the array (copy, move, swap).

Added unit tests.

…the constructor

The callback list of each prototype was created on the first use with double checked
locking, but the first check read the std::shared_ptr without the lock while another
thread could be assigning it under the lock. std::shared_ptr is not atomic, so that's a
data race (ThreadSanitizer reports it when several threads use a fresh HeterCallbackList
or HeterEventDispatcher at the same time).

Now all the lists are created in the constructor (the count of prototypes is a compile time
constant, usually a few), so looking up a list needs no lock at all. A moved-from object
gets new empty lists so it's still usable, as before. callbackListListMutex is kept for the
operations that modify the array (copy, move, swap).

Added unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant