FCV one-definition rule violations

XMLWordPrintableJSON

    • Type: Bug
    • Resolution: Fixed
    • Priority: Major - P3
    • 8.1.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • None
    • Server Programmability
    • Fully Compatible
    • ALL
    • Programmability 2024-11-25
    • 200
    • None
    • 3
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      In a cursory glance at feature compatibility version API (FCV),
      I found a number of unsafe definitions in headers.

      https://github.com/10gen/mongo/blob/master/src/mongo/util/version/releases.h.tpl#L280

      This generates a map in every header that includes util/version/releases.h.
      Every TU will get its own map:

      const std::map<
          FeatureCompatibilityVersion,
          std::pair<FeatureCompatibilityVersion, FeatureCompatibilityVersion>> transitionFCVMap {
          {FeatureCompatibilityVersion::kUpgradingFrom_8_0_To_8_1,
              std::pair{FeatureCompatibilityVersion::kVersion_8_0, FeatureCompatibilityVersion::kVersion_8_1}},
          {FeatureCompatibilityVersion::kDowngradingFrom_8_1_To_8_0,
              std::pair{FeatureCompatibilityVersion::kVersion_8_1, FeatureCompatibilityVersion::kVersion_8_0}},
      };
      
      inline const auto& getTransitionFCVFromAndTo(FeatureCompatibilityVersion v) {
          return transitionFCVMap.at(v);
      }
      

      That also means that every TU will have a getTransitionFCVFromAndTo that refers to a different map. This means that there are multiple definitions of this inline function, all referring to a different object. That's a One-Definition Rule violation.

      I think it's a little hard to see the C++ problems in the source because it's generated from a Cheetah template, but we have to fix it.

      All objects in these FCV files should be audited for ODR problems.

      I believe there are also init-order problems here that can occur if any of these variables are accessed at static init time, and we need to use some care to protect against that.

      Frankly this API should probably have a thin header with some extern definitions and functions, and a .cpp file containing all the tables.

      There are several other suspicious constructs that we can get into when the ticket is picked up.

              Assignee:
              Billy Donahue
              Reporter:
              Billy Donahue
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Created:
                Updated:
                Resolved: