fix possible race condition in Normalizer cache

src/pfaedle/osm/OsmBuilder.cpp

@@ -24,28 +24,28 @@
 #include "util/log/Log.h"
 #include "xml/pfxml.h"
 
-using util::geo::webMercMeterDist;
+using ad::cppgtfs::gtfs::Stop;
-using util::geo::Box;
+using pfaedle::osm::BlockSearch;
-using util::Nullable;
+using pfaedle::osm::EdgeGrid;
-using pfaedle::trgraph::Normalizer;
+using pfaedle::osm::EqSearch;
+using pfaedle::osm::NodeGrid;
+using pfaedle::osm::OsmBuilder;
+using pfaedle::osm::OsmNode;
+using pfaedle::osm::OsmRel;
+using pfaedle::osm::OsmWay;
+using pfaedle::trgraph::Component;
+using pfaedle::trgraph::Edge;
+using pfaedle::trgraph::EdgePL;
 using pfaedle::trgraph::Graph;
 using pfaedle::trgraph::Node;
 using pfaedle::trgraph::NodePL;
-using pfaedle::trgraph::Edge;
+using pfaedle::trgraph::Normalizer;
-using pfaedle::trgraph::EdgePL;
-using pfaedle::trgraph::TransitEdgeLine;
-using pfaedle::trgraph::StatInfo;
 using pfaedle::trgraph::StatGroup;
-using pfaedle::trgraph::Component;
+using pfaedle::trgraph::StatInfo;
-using pfaedle::osm::OsmBuilder;
+using pfaedle::trgraph::TransitEdgeLine;
-using pfaedle::osm::OsmWay;
+using util::Nullable;
-using pfaedle::osm::OsmRel;
+using util::geo::Box;
-using pfaedle::osm::OsmNode;
+using util::geo::webMercMeterDist;
-using pfaedle::osm::EdgeGrid;
-using pfaedle::osm::NodeGrid;
-using pfaedle::osm::EqSearch;
-using pfaedle::osm::BlockSearch;
-using ad::cppgtfs::gtfs::Stop;
 
 // _____________________________________________________________________________
 bool EqSearch::operator()(const Node* cand, const StatInfo* si) const {
@@ -342,7 +342,7 @@ void OsmBuilder::readWriteRels(pfxml::file* i, util::xml::XmlWriter* o,
   OsmRel rel;
   while ((rel = nextRel(i, filter, keepAttrs)).id) {
     OsmIdList realNodes, realWays;
-    std::vector<const char *> realNodeRoles, realWayRoles;
+    std::vector<const char*> realNodeRoles, realWayRoles;
 
     for (size_t j = 0; j < rel.ways.size(); j++) {
       osmid wid = rel.ways[j];
@@ -425,8 +425,8 @@ void OsmBuilder::readWriteWays(pfxml::file* i, util::xml::XmlWriter* o,
 NodePL OsmBuilder::plFromGtfs(const Stop* s, const OsmReadOpts& ops) {
   NodePL ret(
       util::geo::latLngToWebMerc<PFAEDLE_PRECISION>(s->getLat(), s->getLng()),
-      StatInfo(ops.statNormzer(s->getName()),
+      StatInfo(ops.statNormzer.norm(s->getName()),
-               ops.trackNormzer(s->getPlatformCode()), false));
+               ops.trackNormzer.norm(s->getPlatformCode()), false));
 
 #ifdef PFAEDLE_STATION_IDS
   // debug feature, store station id from GTFS
@@ -434,7 +434,8 @@ NodePL OsmBuilder::plFromGtfs(const Stop* s, const OsmReadOpts& ops) {
 #endif
 
   if (s->getParentStation()) {
-    ret.getSI()->addAltName(ops.statNormzer(s->getParentStation()->getName()));
+    ret.getSI()->addAltName(
+        ops.statNormzer.norm(s->getParentStation()->getName()));
   }
 
   return ret;
@@ -442,9 +443,9 @@ NodePL OsmBuilder::plFromGtfs(const Stop* s, const OsmReadOpts& ops) {
 
 // _____________________________________________________________________________
 pfxml::parser_state OsmBuilder::readBBoxNds(pfxml::file* xml, OsmIdSet* nodes,
-                                         OsmIdSet* nohupNodes,
+                                            OsmIdSet* nohupNodes,
-                                         const OsmFilter& filter,
+                                            const OsmFilter& filter,
-                                         const BBoxIdx& bbox) const {
+                                            const BBoxIdx& bbox) const {
   bool inNodeBlock = false;
   uint64_t curId = 0;
 
@@ -739,7 +740,8 @@ void OsmBuilder::readWriteNds(pfxml::file* i, util::xml::XmlWriter* o,
                         {"lat", std::to_string(nd.lat)},
                         {"lon", std::to_string(nd.lng)}});
     for (const auto& kv : nd.attrs) {
-      o->openTag("tag", {{"k", kv.first}, {"v", pfxml::file::decode(kv.second)}});
+      o->openTag("tag",
+                 {{"k", kv.first}, {"v", pfxml::file::decode(kv.second)}});
       o->closeTag();
     }
     o->closeTag();
@@ -937,10 +939,11 @@ std::string OsmBuilder::getAttrByFirstMatch(const DeepAttrLst& rule, osmid id,
                                             const AttrMap& attrs,
                                             const RelMap& entRels,
                                             const RelLst& rels,
-                                            const Normalizer& norm) const {
+                                            const Normalizer& normzer) const {
   std::string ret;
   for (const auto& s : rule) {
-    ret = norm(pfxml::file::decode(getAttr(s, id, attrs, entRels, rels)));
+    ret =
+        normzer.norm(pfxml::file::decode(getAttr(s, id, attrs, entRels, rels)));
     if (!ret.empty()) return ret;
   }
 
@@ -950,11 +953,12 @@ std::string OsmBuilder::getAttrByFirstMatch(const DeepAttrLst& rule, osmid id,
 // _____________________________________________________________________________
 std::vector<std::string> OsmBuilder::getAttrMatchRanked(
     const DeepAttrLst& rule, osmid id, const AttrMap& attrs,
-    const RelMap& entRels, const RelLst& rels, const Normalizer& norm) const {
+    const RelMap& entRels, const RelLst& rels,
+    const Normalizer& normzer) const {
   std::vector<std::string> ret;
   for (const auto& s : rule) {
     std::string tmp =
-        norm(pfxml::file::decode(getAttr(s, id, attrs, entRels, rels)));
+        normzer.norm(pfxml::file::decode(getAttr(s, id, attrs, entRels, rels)));
     if (!tmp.empty()) ret.push_back(tmp);
   }
 
@@ -1183,9 +1187,8 @@ Node* OsmBuilder::depthSearch(const Edge* e, const StatInfo* si, const POINT& p,
 
       int fullTurn = 0;
 
-      if (cur.fromEdge &&
+      if (cur.fromEdge && cur.node->getInDeg() + cur.node->getOutDeg() >
-          cur.node->getInDeg() + cur.node->getOutDeg() >
+                              2) {  // only intersection angles
-              2) {  // only intersection angles
         const POINT& toP = *cand->pl().getGeom();
         const POINT& fromP =
             *cur.fromEdge->getOtherNd(cur.node)->pl().getGeom();
@@ -1425,7 +1428,8 @@ std::vector<TransitEdgeLine*> OsmBuilder::getLines(
       for (const auto& r : ops.relLinerules.sNameRule) {
         for (const auto& relAttr : rels.rels[relId]) {
           if (relAttr.first == r) {
-            el.shortName = ops.lineNormzer(pfxml::file::decode(relAttr.second));
+            el.shortName =
+                ops.lineNormzer.norm(pfxml::file::decode(relAttr.second));
             if (!el.shortName.empty()) found = true;
           }
         }
@@ -1436,7 +1440,8 @@ std::vector<TransitEdgeLine*> OsmBuilder::getLines(
       for (const auto& r : ops.relLinerules.fromNameRule) {
         for (const auto& relAttr : rels.rels[relId]) {
           if (relAttr.first == r) {
-            el.fromStr = ops.statNormzer(pfxml::file::decode(relAttr.second));
+            el.fromStr =
+                ops.statNormzer.norm(pfxml::file::decode(relAttr.second));
             if (!el.fromStr.empty()) found = true;
           }
         }
@@ -1447,7 +1452,8 @@ std::vector<TransitEdgeLine*> OsmBuilder::getLines(
       for (const auto& r : ops.relLinerules.toNameRule) {
         for (const auto& relAttr : rels.rels[relId]) {
           if (relAttr.first == r) {
-            el.toStr = ops.statNormzer(pfxml::file::decode(relAttr.second));
+            el.toStr =
+                ops.statNormzer.norm(pfxml::file::decode(relAttr.second));
             if (!el.toStr.empty()) found = true;
           }
         }
@@ -1895,9 +1901,10 @@ void OsmBuilder::snapStats(const OsmReadOpts& opts, Graph* g,
   }
 
   if (notSnapped.size())
-    LOG(VDEBUG) << notSnapped.size() << " stations could not be snapped in "
+    LOG(VDEBUG) << notSnapped.size()
-                                        "normal run, trying again in orphan "
+                << " stations could not be snapped in "
-                                        "mode.";
+                   "normal run, trying again in orphan "
+                   "mode.";
 
   // try again, but aggressively snap to orphan OSM stations which have
   // not been assigned to any GTFS stop yet

src/pfaedle/router/ShapeBuilder.cpp

@@ -29,28 +29,28 @@
 #include "util/graph/EDijkstra.h"
 #include "util/log/Log.h"
 
-using util::geo::extendBox;
 using util::geo::DBox;
 using util::geo::DPoint;
+using util::geo::extendBox;
 using util::geo::minbox;
 
-using util::geo::webMercMeterDist;
+using ad::cppgtfs::gtfs::ShapePoint;
-using util::geo::webMercToLatLng;
+using ad::cppgtfs::gtfs::Stop;
-using util::geo::latLngToWebMerc;
+using pfaedle::gtfs::Feed;
-using util::geo::output::GeoGraphJsonOutput;
+using pfaedle::gtfs::StopTime;
-using pfaedle::router::ShapeBuilder;
+using pfaedle::gtfs::Trip;
+using pfaedle::osm::BBoxIdx;
+using pfaedle::router::Clusters;
+using pfaedle::router::EdgeListHops;
 using pfaedle::router::FeedStops;
 using pfaedle::router::NodeCandGroup;
 using pfaedle::router::NodeCandRoute;
 using pfaedle::router::RoutingAttrs;
-using pfaedle::router::EdgeListHops;
+using pfaedle::router::ShapeBuilder;
-using pfaedle::router::Clusters;
+using util::geo::latLngToWebMerc;
-using pfaedle::osm::BBoxIdx;
+using util::geo::webMercMeterDist;
-using ad::cppgtfs::gtfs::Stop;
+using util::geo::webMercToLatLng;
-using pfaedle::gtfs::Trip;
+using util::geo::output::GeoGraphJsonOutput;
-using pfaedle::gtfs::Feed;
-using pfaedle::gtfs::StopTime;
-using ad::cppgtfs::gtfs::ShapePoint;
 
 // _____________________________________________________________________________
 ShapeBuilder::ShapeBuilder(Feed* feed, ad::cppgtfs::gtfs::Feed* evalFeed,
@@ -404,16 +404,17 @@ const RoutingAttrs& ShapeBuilder::getRAttrs(const Trip* trip) {
 
     const auto& lnormzer = _motCfg.osmBuildOpts.lineNormzer;
 
-    ret.shortName = lnormzer(trip->getRoute()->getShortName());
+    ret.shortName = lnormzer.norm(trip->getRoute()->getShortName());
-
-    if (ret.shortName.empty()) ret.shortName = lnormzer(trip->getShortname());
 
     if (ret.shortName.empty())
-      ret.shortName = lnormzer(trip->getRoute()->getLongName());
+      ret.shortName = lnormzer.norm(trip->getShortname());
 
-    ret.fromString = _motCfg.osmBuildOpts.statNormzer(
+    if (ret.shortName.empty())
+      ret.shortName = lnormzer.norm(trip->getRoute()->getLongName());
+
+    ret.fromString = _motCfg.osmBuildOpts.statNormzer.norm(
         trip->getStopTimes().begin()->getStop()->getName());
-    ret.toString = _motCfg.osmBuildOpts.statNormzer(
+    ret.toString = _motCfg.osmBuildOpts.statNormzer.norm(
         (--trip->getStopTimes().end())->getStop()->getName());
 
     return _rAttrs
@@ -532,12 +533,12 @@ Clusters ShapeBuilder::clusterTrips(Feed* f, MOTs mots) {
 bool ShapeBuilder::routingEqual(const Stop* a, const Stop* b) {
   if (a == b) return true;  // trivial
 
-  auto namea = _motCfg.osmBuildOpts.statNormzer(a->getName());
+  auto namea = _motCfg.osmBuildOpts.statNormzer.norm(a->getName());
-  auto nameb = _motCfg.osmBuildOpts.statNormzer(b->getName());
+  auto nameb = _motCfg.osmBuildOpts.statNormzer.norm(b->getName());
   if (namea != nameb) return false;
 
-  auto tracka = _motCfg.osmBuildOpts.trackNormzer(a->getPlatformCode());
+  auto tracka = _motCfg.osmBuildOpts.trackNormzer.norm(a->getPlatformCode());
-  auto trackb = _motCfg.osmBuildOpts.trackNormzer(b->getPlatformCode());
+  auto trackb = _motCfg.osmBuildOpts.trackNormzer.norm(b->getPlatformCode());
   if (tracka != trackb) return false;
 
   POINT ap =

src/pfaedle/trgraph/Normalizer.cpp

@@ -3,7 +3,9 @@
 // Authors: Patrick Brosi <brosi@informatik.uni-freiburg.de>
 
 #include <algorithm>
+#include <cassert>
 #include <iostream>
+#include <mutex>
 #include <regex>
 #include <sstream>
 #include <stdexcept>
@@ -15,12 +17,39 @@
 using pfaedle::trgraph::Normalizer;
 
 // _____________________________________________________________________________
-Normalizer::Normalizer(const ReplRules& rules) : _rulesOrig(rules) {
+Normalizer::Normalizer(const ReplRules& rules)
+    : _rulesOrig(rules) {
   buildRules(rules);
 }
 
+// _____________________________________________________________________________
+Normalizer::Normalizer(const Normalizer& other)
+    : _rules(other._rules),
+      _rulesOrig(other._rulesOrig),
+      _cache(other._cache) {}
+
+// _____________________________________________________________________________
+Normalizer& Normalizer::operator=(Normalizer other) {
+  std::swap(this->_rules, other._rules);
+  std::swap(this->_rulesOrig, other._rulesOrig);
+  std::swap(this->_cache, other._cache);
+
+  return *this;
+}
+
 // _____________________________________________________________________________
 std::string Normalizer::operator()(std::string sn) const {
+  return normTS(sn);
+}
+
+// _____________________________________________________________________________
+std::string Normalizer::normTS(const std::string& sn) const {
+  std::lock_guard<std::mutex> lock(_mutex);
+  return norm(sn);
+}
+
+// _____________________________________________________________________________
+std::string Normalizer::norm(const std::string& sn) const {
   auto i = _cache.find(sn);
   if (i != _cache.end()) return i->second;
 

src/pfaedle/trgraph/Normalizer.h

@@ -10,6 +10,7 @@
 #include <unordered_map>
 #include <utility>
 #include <vector>
+#include <mutex>
 
 namespace pfaedle {
 namespace trgraph {
@@ -28,7 +29,19 @@ class Normalizer {
   Normalizer() {}
   explicit Normalizer(const ReplRules& rules);
 
-  // Normalize sn based on the rules of this normalizer
+  // copy constructor
+  Normalizer(const Normalizer& other);
+
+  // assignment op
+  Normalizer& operator=(Normalizer other);
+
+  // Normalize sn, not thread safe
+  std::string norm(const std::string& sn) const;
+  // Normalize sn, thread safe
+  std::string normTS(const std::string& sn) const;
+
+  // Normalize sn based on the rules of this normalizer, uses the thread safe
+  // version of norm() internally
   std::string operator()(std::string sn) const;
   bool operator==(const Normalizer& b) const;
 
@@ -36,6 +49,7 @@ class Normalizer {
   ReplRulesComp _rules;
   ReplRules _rulesOrig;
   mutable std::unordered_map<std::string, std::string> _cache;
+  mutable std::mutex _mutex;
 
   void buildRules(const ReplRules& rules);
 };

src/pfaedle/trgraph/StatGroup.cpp

@@ -68,7 +68,7 @@ double StatGroup::getPen(const Stop* s, trgraph::Node* n,
   double distPen = util::geo::webMercMeterDist(p, *n->pl().getGeom());
   distPen *= distPenFac;
 
-  std::string platform = platformNorm(s->getPlatformCode());
+  std::string platform = platformNorm.norm(s->getPlatformCode());
 
   if (!platform.empty() && !n->pl().getSI()->getTrack().empty() &&
       n->pl().getSI()->getTrack() == platform) {