cpp/openttd-patchpack/source Changeset - r22021:daba722f3935

Changeset - r22021:daba722f3935

Parent rev.

Child rev.

[Not reviewed]

master

0 11 0

frosch - 10 years ago 2015-03-07 18:27:01
frosch@openttd.org

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

11 files changed with 48 insertions and 36 deletions:

src/cheat_gui.cpp

src/linkgraph/linkgraphjob.cpp

src/linkgraph/linkgraphjob.h

src/linkgraph/linkgraphschedule.cpp

src/linkgraph/linkgraphschedule.h

src/linkgraph/mcf.cpp

src/saveload/linkgraph_sl.cpp

src/station.cpp

src/station_base.h

src/station_cmd.cpp

src/toolbar_gui.cpp

0 comments (0 inline, 0 general)

src/cheat_gui.cpp

➞

Show inline comments

@@ @@ -108,7 +108,7 @@ static int32 ClickChangeDateCheat(int32 @@
 	if (p1 == _cur_year) return _cur_year;
 	Date new_date = ConvertYMDToDate(p1, ymd.month, ymd.day);
-	LinkGraphSchedule::Instance()->ShiftDates(new_date - _date);
+	LinkGraphSchedule::instance.ShiftDates(new_date - _date);
 	SetDate(new_date, _date_fract);
 	EnginesMonthlyLoop();
 	SetWindowDirty(WC_STATUS_BAR, 0);

src/linkgraph/linkgraphjob.cpp

➞

Show inline comments

@@ @@ -22,6 +22,13 @@ LinkGraphJobPool _link_graph_job_pool("L @@
 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.

src/linkgraph/linkgraphjob.h

➞

Show inline comments

@@ @@ -343,6 +343,8 @@ public: @@
  */
 class Path {
 public:
 	static Path *invalid_path;
 	Path(NodeID n, bool source = false);
 	/** Get the node this leg passes. */

src/linkgraph/linkgraphschedule.cpp

➞

Show inline comments

@@ @@ -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();
+	}
+}

src/linkgraph/linkgraphschedule.h

➞

Show inline comments

@@ @@ -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();

src/linkgraph/mcf.cpp

➞

Show inline comments

@@ @@ -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(PathVect @@
  */
 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(PathVec @@
 		 * 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;
+	}

src/saveload/linkgraph_sl.cpp

➞

Show inline comments

@@ @@ -220,7 +220,7 @@ static void Load_LGRJ() @@
  */
 static void Load_LGRS()
+{
-	SlObject(LinkGraphSchedule::Instance(), GetLinkGraphScheduleDesc());
+	SlObject(&LinkGraphSchedule::instance, GetLinkGraphScheduleDesc());
+}
 /**
@@ @@ -246,7 +246,7 @@ void AfterLoadLinkGraphs() @@
+		}
+	}
-	LinkGraphSchedule::Instance()->SpawnAll();
+	LinkGraphSchedule::instance.SpawnAll();
+}
 /**
@@ @@ -278,7 +278,7 @@ static void Save_LGRJ() @@
  */
 static void Save_LGRS()
+{
-	SlObject(LinkGraphSchedule::Instance(), GetLinkGraphScheduleDesc());
+	SlObject(&LinkGraphSchedule::instance, GetLinkGraphScheduleDesc());
+}
 /**
@@ @@ -286,7 +286,7 @@ static void Save_LGRS() @@
  */
 static void Ptrs_LGRS()
+{
-	SlObject(LinkGraphSchedule::Instance(), GetLinkGraphScheduleDesc());
+	SlObject(&LinkGraphSchedule::instance, GetLinkGraphScheduleDesc());
+}
 extern const ChunkHandler _linkgraph_chunk_handlers[] = {

src/station.cpp

➞

Show inline comments

@@ @@ -106,7 +106,7 @@ Station::~Station() @@
+		}
 		lg->RemoveNode(this->goods[c].node);
 		if (lg->Size() == 0) {
-			LinkGraphSchedule::Instance()->Unqueue(lg);
+			LinkGraphSchedule::instance.Unqueue(lg);
 			delete lg;
+		}
+	}

src/station_base.h

➞

Show inline comments

@@ @@ -37,6 +37,8 @@ class FlowStat { @@
 public:
 	typedef std::map<uint32, StationID> SharesMap;
 	static const SharesMap empty_sharesmap;
 	/**
 	 * Invalid constructor. This can't be called as a FlowStat must not be
 	 * empty. However, the constructor must be defined and reachable for

src/station_cmd.cpp

➞

Show inline comments

@@ @@ -59,6 +59,13 @@ @@
 #include "safeguards.h"
 /**
  * Static instance of FlowStat::SharesMap.
  * Note: This instance is created on task start.
  *       Lazy creation on first usage results in a data race between the CDist threads.
  */
 /* static */ const FlowStat::SharesMap FlowStat::empty_sharesmap;
 /**
  * Check whether the given tile is a hangar.
  * @param t the tile to of whether it is a hangar.
  * @pre IsTileType(t, MP_STATION)
@@ @@ -3536,7 +3543,7 @@ void IncreaseStats(Station *st, CargoID @@
 		if (ge2.link_graph == INVALID_LINK_GRAPH) {
 			if (LinkGraph::CanAllocateItem()) {
 				lg = new LinkGraph(cargo);
-				LinkGraphSchedule::Instance()->Queue(lg);
+				LinkGraphSchedule::instance.Queue(lg);
 				ge2.link_graph = lg->index;
 				ge2.node = lg->AddNode(st2);
 			} else {
@@ @@ -3558,11 +3565,11 @@ void IncreaseStats(Station *st, CargoID @@
 		if (ge1.link_graph != ge2.link_graph) {
 			LinkGraph *lg2 = LinkGraph::Get(ge2.link_graph);
 			if (lg->Size() < lg2->Size()) {
-				LinkGraphSchedule::Instance()->Unqueue(lg);
+				LinkGraphSchedule::instance.Unqueue(lg);
 				lg2->Merge(lg); // Updates GoodsEntries of lg
 				lg = lg2;
 			} else {
-				LinkGraphSchedule::Instance()->Unqueue(lg2);
+				LinkGraphSchedule::instance.Unqueue(lg2);
 				lg->Merge(lg2); // Updates GoodsEntries of lg2
+			}
+		}
@@ @@ -3684,7 +3691,7 @@ static uint UpdateStationWaiting(Station @@
 	if (ge.link_graph == INVALID_LINK_GRAPH) {
 		if (LinkGraph::CanAllocateItem()) {
 			lg = new LinkGraph(type);
-			LinkGraphSchedule::Instance()->Queue(lg);
+			LinkGraphSchedule::instance.Queue(lg);
 			ge.link_graph = lg->index;
 			ge.node = lg->AddNode(st);
 		} else {

src/toolbar_gui.cpp

➞

Show inline comments

@@ @@ -1125,7 +1125,7 @@ void SetStartingYear(Year year) @@
 	_settings_game.game_creation.starting_year = Clamp(year, MIN_YEAR, MAX_YEAR);
 	Date new_date = ConvertYMDToDate(_settings_game.game_creation.starting_year, 0, 1);
 	/* If you open a savegame as scenario there may already be link graphs.*/
-	LinkGraphSchedule::Instance()->ShiftDates(new_date - _date);
+	LinkGraphSchedule::instance.ShiftDates(new_date - _date);
 	SetDate(new_date, 0);
+}

0 comments (0 inline, 0 general)