[SERVER-70264] Thread-unsafe use of serverGlobalParams Created: 06/Oct/22  Updated: 29/Oct/23  Resolved: 12/Oct/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.2.0-rc0

Type: Bug Priority: Major - P3
Reporter: Billy Donahue Assignee: Matt Diener (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related
is related to SERVER-70448 Create an API for ServerGlobalParams Backlog
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Service Arch 2022-10-17
Participants:
Linked BF Score: 144

 Description   

Hot BF caused by TSAN correctly detecting a data race
in accesses to serverGlobalParams.

https://jira.mongodb.org/browse/BF-26570

From Slack:
https://mongodb.slack.com/archives/CMLKU7Y1M/p1664999003151429

It's just a struct with no synchronization other than what its data members provide for themselves. The place at which the ProfileCmdBase sets these parameters is a sync violation.

The rules for serverGlobalParams are laid out in the .cpp file (it really belongs in the header):

/**
 * This struct represents global configuration data for the server.  These options get set from
 * the command line and are used inline in the code.  Note that much shared code uses this
 * struct, which is why it is here in its own file rather than in the same file as the code that
 * sets it via the command line, which would pull in more dependencies.
 */

Well that's not the whole truth if the ProfileCmd is monkeying with the value. I'd say it's a bug in ProfileCmd that it changes the value of the serverGlobalParams struct without a synchronization fence. It only changes slowMS and sampleRate, so it's not such a big deal to make those Atomic, but I would say that serverGlobalParams should probably obey its docs and never change after the command line is interpreted. Code that relies on those two fields should instead invoke accessor functions. Those functions can get their values in a threadsafe manner by consulting the immutable serverGlobalParams as well as an "optional override variable" that's written to by ProfileCmd.
This is a TSAN true positive and it sounds like a bona fide service arch ticket to me.

ProfileCmd is not the only violator. I think it's just the most recent to be caught by TSAN.
Several tests modify the serverGlobalParams without synchronization.



 Comments   
Comment by Githook User [ 12/Oct/22 ]

Author:

{'name': 'Matt Diener', 'email': 'matt.diener@mongodb.com', 'username': 'mattdiener'}

Message: SERVER-70264 make slowMS and sampleRate atomic
Branch: master
https://github.com/mongodb/mongo/commit/02a79726b9109458d4eb37b010d58faf7332929b

Generated at Thu Feb 08 06:15:43 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.