Skip to content

Commit 001e78a

Browse files
committed
Fix initialization and thread-safety of SDF::Version() global
Replace a static global variable that used a pre-main constructor with a latch-initialized static local variable and mutex to guard the writes. This does not change API but it does change ABI. Signed-off-by: Jeremy Nimmer <jeremy.nimmer@tri.global>
1 parent aaadeea commit 001e78a

File tree

2 files changed

+24
-10
lines changed

2 files changed

+24
-10
lines changed

include/sdf/SDFImpl.hh

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ namespace sdf
236236
public: const std::string &OriginalVersion() const;
237237

238238
/// \brief Get the version
239+
/// The result will be set to SDF_VERSION by default, unless overridden by a
240+
/// call to the Version(const std::string &) function at runtime.
239241
/// \return The version as a string
240242
public: static std::string Version();
241243

@@ -290,10 +292,6 @@ namespace sdf
290292
/// \internal
291293
/// \brief Pointer to private data.
292294
private: std::unique_ptr<SDFPrivate> dataPtr;
293-
294-
/// \brief The SDF version. Set to SDF_VERSION by default, or through
295-
/// the Version function at runtime.
296-
private: static std::string version;
297295
};
298296
/// \}
299297
}

src/SDF.cc

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@
2121
#include <functional>
2222
#include <list>
2323
#include <map>
24+
#include <mutex>
2425
#include <string>
2526
#include <vector>
2627

28+
#include <gz/utils/NeverDestroyed.hh>
29+
2730
#include "sdf/parser.hh"
2831
#include "sdf/Assert.hh"
2932
#include "sdf/Console.hh"
@@ -42,10 +45,6 @@ namespace sdf
4245
{
4346
inline namespace SDF_VERSION_NAMESPACE
4447
{
45-
// TODO(azeey) This violates the Google style guide. Change to a function that
46-
// returns the version string when possible.
47-
std::string SDF::version = SDF_VERSION; // NOLINT(runtime/string)
48-
4948
std::string sdfSharePath()
5049
{
5150
std::string sharePath = sdf::getSharePath();
@@ -488,16 +487,33 @@ const std::string &SDF::OriginalVersion() const
488487
return this->dataPtr->originalVersion;
489488
}
490489

490+
/////////////////////////////////////////////////
491+
namespace {
492+
// Infrastructe to support the SDF::Version(...) static getter and setter.
493+
struct GlobalVersion {
494+
std::mutex mutex;
495+
std::string version{SDF_VERSION};
496+
};
497+
GlobalVersion& GetMutableGlobalVersionSingleton() {
498+
static gz::utils::NeverDestroyed<GlobalVersion> singleton;
499+
return singleton.Access();
500+
}
501+
} // namespace
502+
491503
/////////////////////////////////////////////////
492504
std::string SDF::Version()
493505
{
494-
return version;
506+
GlobalVersion& singleton = GetMutableGlobalVersionSingleton();
507+
std::lock_guard guard{singleton.mutex};
508+
return std::string{singleton.version};
495509
}
496510

497511
/////////////////////////////////////////////////
498512
void SDF::Version(const std::string &_version)
499513
{
500-
version = _version;
514+
GlobalVersion& singleton = GetMutableGlobalVersionSingleton();
515+
std::lock_guard guard{singleton.mutex};
516+
singleton.version = _version;
501517
}
502518

503519
/////////////////////////////////////////////////

0 commit comments

Comments
 (0)