(svn r27178) -Fix [FS#5969]: Data race due to lazy initialisation of objects.

5 files changed, 28 insertions, 25 deletions

diff --git a/src/linkgraph/linkgraphjob.cpp b/src/linkgraph/linkgraphjob.cpp
index 30d68e4d8..20cbf3f64 100644
--- a/src/linkgraph/linkgraphjob.cpp
+++ b/src/linkgraph/linkgraphjob.cpp
@@ -22,6 +22,13 @@ LinkGraphJobPool _link_graph_job_pool("LinkGraphJob");
 INSTANTIATE_POOL_METHODS(LinkGraphJob)
 
 /**
+ * Static instance of an invalid path.
+ * Note: This instance is created on task start.
+ *       Lazy creation on first usage results in a data race between the CDist threads.
+ */
+/* static */ Path *Path::invalid_path = new Path(INVALID_NODE, true);
+
+/**
  * Create a link graph job from a link graph. The link graph will be copied so
  * that the calculations don't interfer with the normal operations on the
  * original. The job is immediately started.
diff --git a/src/linkgraph/linkgraphjob.h b/src/linkgraph/linkgraphjob.h
index 00b65f208..b4587a784 100644
--- a/src/linkgraph/linkgraphjob.h
+++ b/src/linkgraph/linkgraphjob.h
@@ -343,6 +343,8 @@ public:
  */
 class Path {
 public:
+	static Path *invalid_path;
+
 	Path(NodeID n, bool source = false);
 
 	/** Get the node this leg passes. */
diff --git a/src/linkgraph/linkgraphschedule.cpp b/src/linkgraph/linkgraphschedule.cpp
index 30ad20d4d..a65783a5e 100644
--- a/src/linkgraph/linkgraphschedule.cpp
+++ b/src/linkgraph/linkgraphschedule.cpp
@@ -19,6 +19,13 @@
 #include "../safeguards.h"
 
 /**
+ * Static instance of LinkGraphSchedule.
+ * Note: This instance is created on task start.
+ *       Lazy creation on first usage results in a data race between the CDist threads.
+ */
+/* static */ LinkGraphSchedule LinkGraphSchedule::instance;
+
+/**
  * Start the next job in the schedule.
  */
 void LinkGraphSchedule::SpawnNext()
@@ -68,9 +75,8 @@ void LinkGraphSchedule::JoinNext()
 /* static */ void LinkGraphSchedule::Run(void *j)
 {
 	LinkGraphJob *job = (LinkGraphJob *)j;
-	LinkGraphSchedule *schedule = LinkGraphSchedule::Instance();
-	for (uint i = 0; i < lengthof(schedule->handlers); ++i) {
-		schedule->handlers[i]->Run(*job);
+	for (uint i = 0; i < lengthof(instance.handlers); ++i) {
+		instance.handlers[i]->Run(*job);
 	}
 }
 
@@ -90,12 +96,11 @@ void LinkGraphSchedule::SpawnAll()
  */
 /* static */ void LinkGraphSchedule::Clear()
 {
-	LinkGraphSchedule *inst = LinkGraphSchedule::Instance();
-	for (JobList::iterator i(inst->running.begin()); i != inst->running.end(); ++i) {
+	for (JobList::iterator i(instance.running.begin()); i != instance.running.end(); ++i) {
 		(*i)->JoinThread();
 	}
-	inst->running.clear();
-	inst->schedule.clear();
+	instance.running.clear();
+	instance.schedule.clear();
 }
 
 /**
@@ -136,15 +141,6 @@ LinkGraphSchedule::~LinkGraphSchedule()
 }
 
 /**
- * Retrieve the link graph schedule or create it if necessary.
- */
-/* static */ LinkGraphSchedule *LinkGraphSchedule::Instance()
-{
-	static LinkGraphSchedule inst;
-	return &inst;
-}
-
-/**
  * Spawn or join a link graph job or compress a link graph if any link graph is
  * due to do so.
  */
@@ -153,9 +149,9 @@ void OnTick_LinkGraph()
 	if (_date_fract != LinkGraphSchedule::SPAWN_JOIN_TICK) return;
 	Date offset = _date % _settings_game.linkgraph.recalc_interval;
 	if (offset == 0) {
-		LinkGraphSchedule::Instance()->SpawnNext();
+		LinkGraphSchedule::instance.SpawnNext();
 	} else if (offset == _settings_game.linkgraph.recalc_interval / 2) {
-		LinkGraphSchedule::Instance()->JoinNext();
+		LinkGraphSchedule::instance.JoinNext();
 	}
 }
 
diff --git a/src/linkgraph/linkgraphschedule.h b/src/linkgraph/linkgraphschedule.h
index 5d1871962..ec22be316 100644
--- a/src/linkgraph/linkgraphschedule.h
+++ b/src/linkgraph/linkgraphschedule.h
@@ -51,8 +51,8 @@ protected:
 public:
 	/* This is a tick where not much else is happening, so a small lag might go unnoticed. */
 	static const uint SPAWN_JOIN_TICK = 21; ///< Tick when jobs are spawned or joined every day.
+	static LinkGraphSchedule instance;
 
-	static LinkGraphSchedule *Instance();
 	static void Run(void *j);
 	static void Clear();
 
diff --git a/src/linkgraph/mcf.cpp b/src/linkgraph/mcf.cpp
index bd3794a63..6fed7adea 100644
--- a/src/linkgraph/mcf.cpp
+++ b/src/linkgraph/mcf.cpp
@@ -148,15 +148,14 @@ public:
 	 */
 	void SetNode(NodeID source, NodeID node)
 	{
-		static const FlowStat::SharesMap empty;
 		const FlowStatMap &flows = this->job[node].Flows();
 		FlowStatMap::const_iterator it = flows.find(this->job[source].Station());
 		if (it != flows.end()) {
 			this->it = it->second.GetShares()->begin();
 			this->end = it->second.GetShares()->end();
 		} else {
-			this->it = empty.begin();
-			this->end = empty.end();
+			this->it = FlowStat::empty_sharesmap.begin();
+			this->end = FlowStat::empty_sharesmap.end();
 		}
 	}
 
@@ -379,11 +378,10 @@ void MCF1stPass::EliminateCycle(PathVector &path, Path *cycle_begin, uint flow)
  */
 bool MCF1stPass::EliminateCycles(PathVector &path, NodeID origin_id, NodeID next_id)
 {
-	static Path *invalid_path = new Path(INVALID_NODE, true);
 	Path *at_next_pos = path[next_id];
 
 	/* this node has already been searched */
-	if (at_next_pos == invalid_path) return false;
+	if (at_next_pos == Path::invalid_path) return false;
 
 	if (at_next_pos == NULL) {
 		/* Summarize paths; add up the paths with the same source and next hop
@@ -431,7 +429,7 @@ bool MCF1stPass::EliminateCycles(PathVector &path, NodeID origin_id, NodeID next
 		 * could be found in this branch, thus it has to be searched again next
 		 * time we spot it.
 		 */
-		path[next_id] = found ? NULL : invalid_path;
+		path[next_id] = found ? NULL : Path::invalid_path;
 		return found;
 	}