Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
Citizen class is a C++ base class that is designed to help debug memory leaks. Its SDSS predecessor (C, not C++) was apparently incredibly useful, but with our use of smart pointers and RAII, I believe it's essentially been a non-issue. I can't recall a single instance in which it was helpful in debugging a memory problem, and it's the source of a constant trickle of confusion due to failures to call Citizen::markPersistent and the inherited getId method looking like it returns some other kind of ID.
I think we should at least #define out the Citizen implementation by default. That'll make it truly a zero-cost utility in non-debug builds, and let us use it in more places (right now, we don't use it for small classes, which decreases its utility even further). If we do that, I'd also advocate changing the interface to only provide access via a separate class (see DM-675).
But I think we could save the most effort by just removing it entirely, as that'll save on future maintenance effort (such as modifying the threading stuff to use C++11 constructs, or just making sure it works properly with pybind11, which is pickier than Swig about returning raw pointers). I'd like to specifically ping Robert Lupton on that question (as well as the broader usual audience for RFCs) - I know he agrees that Citizen isn't as useful as it was in C, but I'm very interested in his thoughts about removing it entirely.
I'd also be interested to hear from people coming from other C++ projects as to whether they used (or wish they'd had) similar classes.
Jim Bosch this RFC has bubbled up again.