Skip to content
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

Fix crash due to concurrent r&w #4619

Closed
wants to merge 2 commits into from
Closed

Fix crash due to concurrent r&w #4619

wants to merge 2 commits into from

Conversation

tomitheninja
Copy link

I use AdGuardHomeExporter, which fetches the latest metrics via https every 10 seconds.
I noticed that AGH crashes frequently (four times today), due to a concurrent map iteration and map write.
This happens when I try to fetch the metrics, while it is getting updated.
I noticed the statsCtx has a mutex, that is not being used.

Fixes #4342 #4374

I use AdGuardHomeExporter, which fetches the latest metrics via https every 10 seconds.
I noticed that AGH crashes frequently (four times today), due to a concurrent map iteration and map write.
This happens when I try to fetch the metrics, while it is getting updated.
I noticed the statsCtx has a mutex, that is not being used.

Fixes #4342 #4374
@tomitheninja tomitheninja changed the title Fix crash due to concurrent r&w bug Fix crash due to concurrent r&w Jun 2, 2022
@ainar-g ainar-g self-assigned this Jun 3, 2022
@ainar-g
Copy link
Contributor

ainar-g commented Jun 3, 2022

Thanks for the contribution, but that will probably not be enough to truly fix the issue. For example, there may still be a race between that call of loadUnits and the one in GetTopClientsIP. Also, this looks like it could cause a deadlock in ongoing, which loadUnits calls.

I plan on fixing all these races as well as some other stats-related bugs in the next release, but feel free to investigate as well.

@EugeneOne1
Copy link
Member

EugeneOne1 commented Aug 4, 2022

@tomitheninja, we've improve the concurrency logic in the stats module as per 4293cf5. The latest build in the edge channel already contains the fix. Could you please install it (or build AGH from master branch) and tell if the concurrency issues are still occuring?

@EugeneOne1 EugeneOne1 added the waiting for data Waiting for users to provide more data. label Aug 22, 2022
@ainar-g
Copy link
Contributor

ainar-g commented Aug 25, 2022

We'll close this PR for now. Please feel free to file new issues or reopen this PR if you find other bugs.

@ainar-g ainar-g closed this Aug 25, 2022
@ainar-g ainar-g added duplicate Duplicate or merged issues. and removed waiting for data Waiting for users to provide more data. labels Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Duplicate or merged issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error: concurrent map iteration and map write
3 participants