Skip to content

TSan: Fix data race of g_num_records - #9170

Merged
masaori335 merged 1 commit into
apache:masterfrom
masaori335:tsan-2
Nov 7, 2022
Merged

TSan: Fix data race of g_num_records#9170
masaori335 merged 1 commit into
apache:masterfrom
masaori335:tsan-2

Conversation

@masaori335

@masaori335 masaori335 commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Fix below TSan report.

WARNING: ThreadSanitizer: data race (pid=2377)
  Read of size 4 at 0x00001020f720 by thread T18 (mutexes: write M0, write M1):
    #0 RecExecConfigUpdateCbs(unsigned int) P_RecCore.cc:658 (traffic_server:x86_64+0x5042db) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #1 config_update_cont::exec_callbacks(int, Event*) RecProcess.cc:161 (traffic_server:x86_64+0x51cb6c) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #2 EThread::process_event(Event*, int) UnixEThread.cc:153 (traffic_server:x86_64+0x52283b) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #3 EThread::execute_regular() UnixEThread.cc:258 (traffic_server:x86_64+0x52379f) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #4 EThread::execute() UnixEThread.cc:349 (traffic_server:x86_64+0x5241d9) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #5 spawn_thread_internal(void*) Thread.cc:79 (traffic_server:x86_64+0x521258) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)

  Previous atomic write of size 4 at 0x00001020f720 by thread T8 (mutexes: write M2, write M3, write M4):
    #0 RecAlloc(RecT, char const*, RecDataT) RecUtils.cc:58 (traffic_server:x86_64+0x5190b9) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #1 register_record(RecT, char const*, RecDataT, RecData, RecPersistT, bool*) RecCore.cc:88 (traffic_server:x86_64+0x50ade4) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #2 RecRegisterStat(RecT, char const*, RecDataT, RecData, RecPersistT) RecCore.cc:865 (traffic_server:x86_64+0x50aae5) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #3 _RecRegisterRawStat(RecRawStatBlock*, RecT, char const*, RecDataT, RecPersistT, int, int (*)(char const*, RecDataT, RecData*, RecRawStatBlock*, int)) RecRawStats.cc:262 (traffic_server:x86_64+0x5161de) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #4 register_cache_stats(RecRawStatBlock*, char const*) Cache.cc:3136 (traffic_server:x86_64+0x35adfb) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #5 CacheProcessor::diskInitialized() Cache.cc:822 (traffic_server:x86_64+0x352c5c) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #6 CacheDisk::openStart(int, void*) CacheDisk.cc:208 (traffic_server:x86_64+0x380309) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #7 AIOCallbackInternal::io_complete(int, void*) P_AIO.h:122 (traffic_server:x86_64+0x3d7dad) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #8 EThread::process_event(Event*, int) UnixEThread.cc:153 (traffic_server:x86_64+0x52283b) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #9 EThread::process_queue(Queue<Event, Event::Link_link>*, int*, int*) UnixEThread.cc:188 (traffic_server:x86_64+0x522f6f) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #10 EThread::execute_regular() UnixEThread.cc:244 (traffic_server:x86_64+0x523721) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #11 EThread::execute() UnixEThread.cc:349 (traffic_server:x86_64+0x5241d9) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)
    #12 spawn_thread_internal(void*) Thread.cc:79 (traffic_server:x86_64+0x521258) (BuildId: fd5ac86be47a3d099e1161ab8724ba5432000000200000000100000000000c00)

  Location is global 'g_num_records' at 0x00001020f720 (traffic_server+0x746720)

It looks like the g_records_rwlock is the guard of access to the g_records, g_records_ht, and g_num_records, (global variables and shared across threads).

RecRecord *g_records = nullptr;
std::unordered_map<std::string, RecRecord *> g_records_ht;
ink_rwlock g_records_rwlock;
int g_num_records = 0;

  1. RecAlloc() is always called under the write lock of g_records_rwlock. We don't need atomic increment.
  2. RecExecConfigUpdateCbs() needs the read lock of g_records_rwlock to refer the g_num_records and g_records.

@masaori335 masaori335 added the TSan label Nov 2, 2022
@masaori335 masaori335 added this to the 10.0.0 milestone Nov 2, 2022
@masaori335 masaori335 self-assigned this Nov 2, 2022
@bryancall
bryancall requested a review from ywkaras November 2, 2022 23:12
@masaori335
masaori335 merged commit c32399c into apache:master Nov 7, 2022
SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants