Skip to content

cmap: Fix data race reported by ThreadSanitizer #22

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.

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
merged 1 commit into from
Sep 1, 2023

Conversation

RinHizakura
Copy link
Contributor

There are some data races on the cmap example, and this pull request tries to fix it.

Most of the race can be simply fixed by applying atomic operation. For example, the synchronization on the counter or status variables. However, there are still fixes that may have to be considered carefully. As the behavior may be different from the original implementation.

One of them is the race of random seed. In my opinion, I think it should make more sense to maintain a thread-independent seed for each thread. Thus the seed is changed to a thread local storage variable.

Another hard problem is the race between the removed part of the writer thread and the other reader threads. It is dangerous because the writer could free the memory that is still used by readers, which causes use-after-free bugs. This is not a simple question. If you want an elegant and even efficient solution, it may be ideal to introduce some high-level data structure such as the hazard pointer. But considering that this project should just provide an easy-to-use example, I choose a simple and crude method: maintain a linked list of memory that should be released, and not release it until the end of the program. Although this is obviously not a smart way, it is at least the simplest way to ensure correctness.

@jserv jserv changed the title Fix data race issues detected by TSAN for cmap cmap: Fix data race reported by ThreadSanitizer Aug 31, 2023
@jserv jserv merged commit a7ccb3e into sysprog21:master Sep 1, 2023
@jserv
Copy link
Contributor

jserv commented Sep 1, 2023

Thank @RinHizakura for contributing!

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.

2 participants