Follow-up from issue #116 (review finding #8).
xmscore/misc/Singleton.h:25-69 — Singleton<T>::Instance(bool a_delete, T* a_new) overloads three responsibilities (get / install / destroy) onto one entry point and, on the destruction path, returns a reference to the payload of an already-emptied shared_ptr (undefined behavior).
```cpp
static T& Instance(bool a_delete, T* a_new) {
...
return *theSingleInstance.get(); // a_delete=true: dereferences empty shared_ptr
}
```
Proposed change
- Split the API into:
Instance() — returns the live instance (lazy-init or assert).SetInstance(std::unique_ptr<T>) — install a replacement; document and enforce what happens if one is already installed.Reset() — tear down; returns void (or bool), never a dangling T&.
- Mark the legacy combined
Instance(bool, T*) [[deprecated]] and migrate call sites in this repo (and downstream xms libs) before removal.
Why follow-up, not part of #117
The doc-only PR for #116 freezes "CRTP Singleton" as the documented contract via the new \brief. Fixing the type behind the docs is its own structural change that needs its own review pass and downstream impact analysis.
Acceptance
Refs: issue #116, PR #117.
Follow-up from issue #116 (review finding #8).
xmscore/misc/Singleton.h:25-69—Singleton<T>::Instance(bool a_delete, T* a_new)overloads three responsibilities (get / install / destroy) onto one entry point and, on the destruction path, returns a reference to the payload of an already-emptiedshared_ptr(undefined behavior).```cpp
static T& Instance(bool a_delete, T* a_new) {
...
return *theSingleInstance.get(); // a_delete=true: dereferences empty shared_ptr
}
```
Proposed change
Instance()— returns the live instance (lazy-init or assert).SetInstance(std::unique_ptr<T>)— install a replacement; document and enforce what happens if one is already installed.Reset()— tear down; returnsvoid(orbool), never a danglingT&.Instance(bool, T*)[[deprecated]]and migrate call sites in this repo (and downstream xms libs) before removal.Why follow-up, not part of #117
The doc-only PR for #116 freezes "CRTP Singleton" as the documented contract via the new
\brief. Fixing the type behind the docs is its own structural change that needs its own review pass and downstream impact analysis.Acceptance
Instance()/SetInstance()/Reset()exist with the contracts aboveshared_ptr's payloadInstance(bool, T*)marked deprecated with a migration noteRefs: issue #116, PR #117.