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

Protect shared variables with log lock. #13306

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

vkalintiris
Copy link
Collaborator

@vkalintiris vkalintiris commented Jul 5, 2022

Summary

Fixes reports by helgrind:

==00:00:00:01.769 44512== Possible data race during read of size 8 at 0x9767B0 by thread https://github.com/netdata/netdata/pull/4
==00:00:00:01.769 44512== Locks held: none
==00:00:00:01.769 44512==    at 0x17CB56: error_log_limit (log.c:627)
==00:00:00:01.769 44512==    by 0x17CEC0: info_int (log.c:716)
==00:00:00:01.769 44512==    by 0x18949F: thread_start (threads.c:173)
==00:00:00:01.769 44512==    by 0x484A486: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:01.769 44512==    by 0x4E9CD7F: start_thread (pthread_create.c:481)
==00:00:00:01.769 44512==    by 0x532F76E: clone (clone.S:95)
==00:00:00:01.769 44512==
==00:00:00:01.769 44512== This conflicts with a previous write of size 8 by thread https://github.com/netdata/netdata/pull/3
==00:00:00:01.769 44512== Locks held: none
==00:00:00:01.769 44512==    at 0x17CB61: error_log_limit (log.c:627)
==00:00:00:01.769 44512==    by 0x17CEC0: info_int (log.c:716)
==00:00:00:01.769 44512==    by 0x18949F: thread_start (threads.c:173)
==00:00:00:01.769 44512==    by 0x484A486: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:01.769 44512==    by 0x4E9CD7F: start_thread (pthread_create.c:481)
==00:00:00:01.769 44512==    by 0x532F76E: clone (clone.S:95)
==00:00:00:01.769 44512==  Address 0x9767b0 is 0 bytes inside data symbol "counter.1"
==00:00:00:44.536 47685==  Lock at 0x976720 was first observed
==00:00:00:44.536 47685==    at 0x48477EF: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:44.536 47685==    by 0x17BBF4: __netdata_mutex_lock (locks.c:86)
==00:00:00:44.536 47685==    by 0x17C514: log_lock (log.c:471)
==00:00:00:44.536 47685==    by 0x17CEC0: info_int (log.c:715)
==00:00:00:44.536 47685==    by 0x458C9E: compute_multidb_diskspace (rrdenginelib.c:279)
==00:00:00:44.536 47685==    by 0x15B170: get_netdata_configured_variables (main.c:671)
==00:00:00:44.536 47685==    by 0x15CE6C: main (main.c:1263)
==00:00:00:44.536 47685==  Address 0x976720 is 0 bytes inside data symbol "log_mutex"
==00:00:00:44.536 47685==
==00:00:00:44.536 47685== Possible data race during write of size 8 at 0x9767A0 by thread https://github.com/netdata/netdata/issues/1
==00:00:00:44.536 47685== Locks held: none
==00:00:00:44.536 47685==    at 0x17CB39: error_log_limit (log.c:621)
==00:00:00:44.536 47685==    by 0x15E234: signals_handle (signals.c:258)
==00:00:00:44.536 47685==    by 0x15D880: main (main.c:1534)
==00:00:00:44.536 47685==
==00:00:00:44.536 47685== This conflicts with a previous read of size 8 by thread https://github.com/netdata/netdata/pull/9
==00:00:00:44.536 47685== Locks held: 1, at address 0x976720
==00:00:00:44.536 47685==    at 0x17CAA3: error_log_limit (log.c:604)
==00:00:00:44.536 47685==    by 0x17CECA: info_int (log.c:718)
==00:00:00:44.536 47685==    by 0x4624D2: rrdset_done_push (rrdpush.c:344)
==00:00:00:44.536 47685==    by 0x36190C: rrdset_done (rrdset.c:1351)
==00:00:00:44.536 47685==    by 0x1B07E7: Chart::update(unsigned long) (plugin_profile.cc:82)
==00:00:00:44.536 47685==    by 0x1B01D4: updateCharts(std::vector<Chart*, std::allocator<Chart*> >, unsigned long) (plugin_profile.cc:126)
==00:00:00:44.536 47685==    by 0x1B02AC: profile_main (plugin_profile.cc:144)
==00:00:00:44.536 47685==    by 0x1895D4: thread_start (threads.c:185)
==00:00:00:44.536 47685==  Address 0x9767a0 is 0 bytes inside data symbol "start.3"
Test Plan

Run with/without this patch the agent under helgrind and check the logs.

Additional Information

#13298

@vkalintiris vkalintiris requested a review from thiagoftsm as a code owner July 5, 2022 10:39
@vkalintiris vkalintiris self-assigned this Jul 5, 2022
@vkalintiris vkalintiris marked this pull request as draft July 5, 2022 10:59
Fixes reports by helgrind:

```
==00:00:00:01.769 44512== Possible data race during read of size 8 at 0x9767B0 by thread #4
==00:00:00:01.769 44512== Locks held: none
==00:00:00:01.769 44512==    at 0x17CB56: error_log_limit (log.c:627)
==00:00:00:01.769 44512==    by 0x17CEC0: info_int (log.c:716)
==00:00:00:01.769 44512==    by 0x18949F: thread_start (threads.c:173)
==00:00:00:01.769 44512==    by 0x484A486: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:01.769 44512==    by 0x4E9CD7F: start_thread (pthread_create.c:481)
==00:00:00:01.769 44512==    by 0x532F76E: clone (clone.S:95)
==00:00:00:01.769 44512==
==00:00:00:01.769 44512== This conflicts with a previous write of size 8 by thread #3
==00:00:00:01.769 44512== Locks held: none
==00:00:00:01.769 44512==    at 0x17CB61: error_log_limit (log.c:627)
==00:00:00:01.769 44512==    by 0x17CEC0: info_int (log.c:716)
==00:00:00:01.769 44512==    by 0x18949F: thread_start (threads.c:173)
==00:00:00:01.769 44512==    by 0x484A486: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:01.769 44512==    by 0x4E9CD7F: start_thread (pthread_create.c:481)
==00:00:00:01.769 44512==    by 0x532F76E: clone (clone.S:95)
==00:00:00:01.769 44512==  Address 0x9767b0 is 0 bytes inside data symbol "counter.1"
```

```
==00:00:00:44.536 47685==  Lock at 0x976720 was first observed
==00:00:00:44.536 47685==    at 0x48477EF: ??? (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==00:00:00:44.536 47685==    by 0x17BBF4: __netdata_mutex_lock (locks.c:86)
==00:00:00:44.536 47685==    by 0x17C514: log_lock (log.c:471)
==00:00:00:44.536 47685==    by 0x17CEC0: info_int (log.c:715)
==00:00:00:44.536 47685==    by 0x458C9E: compute_multidb_diskspace (rrdenginelib.c:279)
==00:00:00:44.536 47685==    by 0x15B170: get_netdata_configured_variables (main.c:671)
==00:00:00:44.536 47685==    by 0x15CE6C: main (main.c:1263)
==00:00:00:44.536 47685==  Address 0x976720 is 0 bytes inside data symbol "log_mutex"
==00:00:00:44.536 47685==
==00:00:00:44.536 47685== Possible data race during write of size 8 at 0x9767A0 by thread #1
==00:00:00:44.536 47685== Locks held: none
==00:00:00:44.536 47685==    at 0x17CB39: error_log_limit (log.c:621)
==00:00:00:44.536 47685==    by 0x15E234: signals_handle (signals.c:258)
==00:00:00:44.536 47685==    by 0x15D880: main (main.c:1534)
==00:00:00:44.536 47685==
==00:00:00:44.536 47685== This conflicts with a previous read of size 8 by thread #9
==00:00:00:44.536 47685== Locks held: 1, at address 0x976720
==00:00:00:44.536 47685==    at 0x17CAA3: error_log_limit (log.c:604)
==00:00:00:44.536 47685==    by 0x17CECA: info_int (log.c:718)
==00:00:00:44.536 47685==    by 0x4624D2: rrdset_done_push (rrdpush.c:344)
==00:00:00:44.536 47685==    by 0x36190C: rrdset_done (rrdset.c:1351)
==00:00:00:44.536 47685==    by 0x1B07E7: Chart::update(unsigned long) (plugin_profile.cc:82)
==00:00:00:44.536 47685==    by 0x1B01D4: updateCharts(std::vector<Chart*, std::allocator<Chart*> >, unsigned long) (plugin_profile.cc:126)
==00:00:00:44.536 47685==    by 0x1B02AC: profile_main (plugin_profile.cc:144)
==00:00:00:44.536 47685==    by 0x1895D4: thread_start (threads.c:185)
==00:00:00:44.536 47685==  Address 0x9767a0 is 0 bytes inside data symbol "start.3"
```
@vkalintiris vkalintiris marked this pull request as ready for review July 6, 2022 09:14
Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this PR locally and I did not observe more the errors using valgrind-3.17.

@vkalintiris vkalintiris merged commit 151a603 into netdata:master Jul 7, 2022
@vkalintiris vkalintiris deleted the log-lock branch July 5, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants