[SERVER-17482] Benign race in LockHead::newRequest() Created: 05/Mar/15  Updated: 14/Apr/16  Resolved: 24/Apr/15

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Eric Milkie Assignee: Geert Bosch
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Steps To Reproduce:

WARNING: ThreadSanitizer: data race (pid=1025)
  Read of size 4 at 0x7d780000d8c8 by main thread:
    #0 mongo::LockManager::unlock(mongo::LockRequest*) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:570 (mongod+0x000001202c0b)
    #1 mongo::LockerImpl<true>::_unlockImpl(mongo::FastMapNoAlloc<mongo::ResourceId, mongo::LockRequest, 16>::IteratorImpl<mongo::FastMapNoAlloc<mongo::ResourceId, mongo::LockRequest, 16>, mongo::LockRequest>&) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:764 (mongod+0x00000122ca73)
    #2 mongo::LockerImpl<true>::unlock(mongo::ResourceId) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.h:434 (mongod+0x00000122d707)
    #3 mongo::LockerImpl<true>::endWriteUnitOfWork() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:394 (mongod+0x00000122cf22)
    #4 mongo::WriteUnitOfWork::commit() /home/rassi/work/mongo/src/mongo/db/operation_context.h:167 (mongod+0x000000d07143)
    #5 mongo::MMAPV1DatabasCatalogEntry::_init(mongo::OperationContext*) /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp:604 (mongod+0x000001c7befb)
    #6 mongo::MMAPV1DatabaseCatalogEntry::MMAPV1DatabaseCatalogEntry(mongo::OperationContext*, mongo::StringData, mongo::StringData, bool, bool) /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp:185 (mongod+0x000001c79cb9)
    #7 mongo::MMAPV1Engine::getDatabaseCatalogEntry(mongo::OperationContext*, mongo::StringData) /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp:267 (mongod+0x000001c9275a)
    #8 mongo::DatabaseHolder::openDb(mongo::OperationContext*, mongo::StringData, bool*) /home/rassi/work/mongo/src/mongo/db/catalog/database_holder.cpp:128 (mongod+0x000001055b20)
    #9 mongo::repairDatabasesAndCheckVersion() /home/rassi/work/mongo/src/mongo/db/db.cpp:372 (mongod+0x000000cf3f91)
    #10 mongo::_initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:538 (mongod+0x000000cefbab)
    #11 mongo::initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:639 (mongod+0x000000cee05c)
    #12 mongoDbMain(int, char**, char**) /home/rassi/work/mongo/src/mongo/db/db.cpp:885 (mongod+0x000000cf13a7)
    #13 main /home/rassi/work/mongo/src/mongo/db/db.cpp:688 (mongod+0x000000cf0d9d)
 
  Previous write of size 4 at 0x7d780000d8c8 by thread T4 (mutexes: write M1347, write M40, write M167):
    #0 mongo::LockHead::newRequest(mongo::LockRequest*, mongo::LockMode) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:219 (mongod+0x0000012094d8)
    #1 mongo::LockHead::migratePartitionedLockHeads() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:396 (mongod+0x0000011ffbd8)
    #2 mongo::LockManager::lock(mongo::ResourceId, mongo::LockRequest*, mongo::LockMode) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:479 (mongod+0x00000120142d)
    #3 mongo::LockerImpl<true>::lockBegin(mongo::ResourceId, mongo::LockMode) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.h:665 (mongod+0x00000122a68a)
    #4 mongo::LockerImpl<true>::lock(mongo::ResourceId, mongo::LockMode, unsigned int, bool) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:413 (mongod+0x00000122d3ab)
    #5 mongo::AutoAcquireFlushLockForMMAPV1Commit::AutoAcquireFlushLockForMMAPV1Commit(mongo::Locker*) /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:838 (mongod+0x000001225d68)
    #6 mongo::dur::durThread() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:737 (mongod+0x000001c14bcd)
    #7 boost::detail::thread_data<void (*)()>::run() /home/rassi/work/mongo/src/third_party/boost-1.56.0/boost/thread/detail/thread.hpp:115 (mongod+0x000001958f98)
    #8 boost::(anonymous namespace)::thread_proxy(void*) /home/rassi/work/mongo/src/third_party/boost-1.56.0/libs/thread/src/pthread/thread.cpp:173 (mongod+0x000002297af3)
 
  Location is heap block of size 2936 at 0x7d780000d800 allocated by main thread:
    #0 operator new(unsigned long) <null>:0 (mongod+0x000000c9269d)
    #1 mongo::(anonymous namespace)::newLocker() /home/rassi/work/mongo/src/mongo/db/client.cpp:82 (mongod+0x00000108c4b1)
    #2 mongo::Client::getLocker() /home/rassi/work/mongo/src/mongo/db/client.cpp:331 (mongod+0x00000108c44e)
    #3 mongo::OperationContextImpl::OperationContextImpl() /home/rassi/work/mongo/src/mongo/db/operation_context_impl.cpp:52 (mongod+0x0000015c48a3)
    #4 mongo::dur::replayJournalFilesAtStartup() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur_recover.cpp:605 (mongod+0x000001c496f0)
    #5 mongo::dur::startup() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:898 (mongod+0x000001c13bc6)
    #6 mongo::MMAPV1Engine::finishInit() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp:235 (mongod+0x000001c91658)
    #7 mongo::GlobalEnvironmentMongoD::setGlobalStorageEngine(std::string const&) /home/rassi/work/mongo/src/mongo/db/global_environment_d.cpp:108 (mongod+0x0000014b4cfa)
    #8 mongo::_initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:464 (mongod+0x000000ceedf1)
    #9 mongo::initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:639 (mongod+0x000000cee05c)
    #10 mongoDbMain(int, char**, char**) /home/rassi/work/mongo/src/mongo/db/db.cpp:885 (mongod+0x000000cf13a7)
    #11 main /home/rassi/work/mongo/src/mongo/db/db.cpp:688 (mongod+0x000000cf0d9d)
 
  Mutex M1347 created at:
    #0 pthread_mutex_init <null>:0 (mongod+0x000000c961e0)
    #1 boost::mutex::mutex() /home/rassi/work/mongo/src/third_party/boost-1.56.0/boost/thread/pthread/mutex.hpp:101 (mongod+0x000000d14ced)
    #2 __cxx_global_var_init15 /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:116 (mongod+0x000000c4a70c)
    #3 _GLOBAL__I_a /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:354 (mongod+0x000000c4aa1c)
    #4 __libc_csu_init <null>:0 (mongod+0x00000356130c)
 
  Mutex M40 created at:
    #0 pthread_mutex_init <null>:0 (mongod+0x000000c961e0)
    #1 mongo::SimpleMutex::SimpleMutex(mongo::StringData) /home/rassi/work/mongo/src/mongo/util/concurrency/mutex.h:157 (mongod+0x000001029450)
    #2 mongo::LockManager::LockBucket::LockBucket() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.h:133 (mongod+0x0000012097d2)
    #3 mongo::LockManager::LockManager() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:420 (mongod+0x0000011ffe47)
    #4 __cxx_global_var_init40 /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:116 (mongod+0x000000c0222c)
    #5 _GLOBAL__I_a /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:354 (mongod+0x000000c0259a)
    #6 __libc_csu_init <null>:0 (mongod+0x00000356130c)
 
  Mutex M167 created at:
    #0 pthread_mutex_init <null>:0 (mongod+0x000000c961e0)
    #1 mongo::SimpleMutex::SimpleMutex(mongo::StringData) /home/rassi/work/mongo/src/mongo/util/concurrency/mutex.h:157 (mongod+0x000001029450)
    #2 mongo::LockManager::Partition::Partition() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.h:144 (mongod+0x000001209992)
    #3 mongo::LockManager::LockManager() /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:421 (mongod+0x0000011ffed6)
    #4 __cxx_global_var_init40 /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:116 (mongod+0x000000c0222c)
    #5 _GLOBAL__I_a /home/rassi/work/mongo/src/mongo/db/concurrency/lock_state.cpp:354 (mongod+0x000000c0259a)
    #6 __libc_csu_init <null>:0 (mongod+0x00000356130c)
 
  Thread T4 (tid=1032, running) created by main thread at:
    #0 pthread_create <null>:0 (mongod+0x000000c95b51)
    #1 boost::thread::start_thread_noexcept() /home/rassi/work/mongo/src/third_party/boost-1.56.0/libs/thread/src/pthread/thread.cpp:255 (mongod+0x0000022979a6)
    #2 boost::thread::start_thread() /home/rassi/work/mongo/src/third_party/boost-1.56.0/boost/thread/detail/thread.hpp:178 (mongod+0x000000d0f3d0)
    #3 boost::thread::thread<void (&)()>(void (&)()) /home/rassi/work/mongo/src/third_party/boost-1.56.0/boost/thread/detail/thread.hpp:265 (mongod+0x0000019580cf)
    #4 mongo::dur::(anonymous namespace)::DurableImpl::start() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:620 (mongod+0x000001c141bf)
    #5 mongo::dur::startup() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/dur.cpp:915 (mongod+0x000001c14037)
    #6 mongo::MMAPV1Engine::finishInit() /home/rassi/work/mongo/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp:235 (mongod+0x000001c91658)
    #7 mongo::GlobalEnvironmentMongoD::setGlobalStorageEngine(std::string const&) /home/rassi/work/mongo/src/mongo/db/global_environment_d.cpp:108 (mongod+0x0000014b4cfa)
    #8 mongo::_initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:464 (mongod+0x000000ceedf1)
    #9 mongo::initAndListen(int) /home/rassi/work/mongo/src/mongo/db/db.cpp:639 (mongod+0x000000cee05c)
    #10 mongoDbMain(int, char**, char**) /home/rassi/work/mongo/src/mongo/db/db.cpp:885 (mongod+0x000000cf13a7)
    #11 main /home/rassi/work/mongo/src/mongo/db/db.cpp:688 (mongod+0x000000cf0d9d)
 
SUMMARY: ThreadSanitizer: data race /home/rassi/work/mongo/src/mongo/db/concurrency/lock_manager.cpp:570 mongo::LockManager::unlock(mongo::LockRequest*)

Sprint: Quint Iteration 3.1.2
Participants:

 Description   

When migratePartitionedLockHeads() calls newRequest(), it can write to member variables (LockRequest::mode, LockRequest::status) that are owned by another thread with no synchronization. This is benign because the values written are guaranteed to be the same as the preexisting values, by the code logic. However, this still trips a data race error from the thread sanitizer.
Kal recommends that we write a new function to supplement newRequest() that includes only the logic that is necessary for migratePartitionedLockHeads().



 Comments   
Comment by Geert Bosch [ 24/Apr/15 ]

This is not an actual data race. In particular, at all times the lock request is either protected by the partition mutex or by the lock manager bucket mutex. Which of the two is determined by the "partitioned" flag. The only situation where the lock request protection changes is in migratePartitionedLockHeads. However, as can be easily seen, migratePartitionedLockHeads always holds both the partition mutex and the bucket mutex when migrating.

I think the analyzer just gets confused because of the transfer of ownership. So we just have to fix this false positive.

Generated at Thu Feb 08 03:44:38 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.