summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfrosch <frosch@openttd.org>2015-03-07 18:27:01 +0000
committerfrosch <frosch@openttd.org>2015-03-07 18:27:01 +0000
commit780e5959331b39eda19868126af41398a904818b (patch)
treee2d592a1b2ec24af7ae1190c9ef970289b2e8e03
parente77de936366fd4a0773386b8bdbe5bd6da354c89 (diff)
downloadopenttd-780e5959331b39eda19868126af41398a904818b.tar.xz
(svn r27178) -Fix [FS#5969]: Data race due to lazy initialisation of objects.
-rw-r--r--src/cheat_gui.cpp2
-rw-r--r--src/linkgraph/linkgraphjob.cpp7
-rw-r--r--src/linkgraph/linkgraphjob.h2
-rw-r--r--src/linkgraph/linkgraphschedule.cpp32
-rw-r--r--src/linkgraph/linkgraphschedule.h2
-rw-r--r--src/linkgraph/mcf.cpp10
-rw-r--r--src/saveload/linkgraph_sl.cpp8
-rw-r--r--src/station.cpp2
-rw-r--r--src/station_base.h2
-rw-r--r--src/station_cmd.cpp15
-rw-r--r--src/toolbar_gui.cpp2
11 files changed, 48 insertions, 36 deletions
diff --git a/src/cheat_gui.cpp b/src/cheat_gui.cpp
index e252912c3..fda2b246e 100644
--- a/src/cheat_gui.cpp
+++ b/src/cheat_gui.cpp
@@ -108,7 +108,7 @@ static int32 ClickChangeDateCheat(int32 p1, int32 p2)
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);
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;
}
diff --git a/src/saveload/linkgraph_sl.cpp b/src/saveload/linkgraph_sl.cpp
index 20a3f4498..a65f4fc8a 100644
--- a/src/saveload/linkgraph_sl.cpp
+++ b/src/saveload/linkgraph_sl.cpp
@@ -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[] = {
diff --git a/src/station.cpp b/src/station.cpp
index e0a48ef22..456262dea 100644
--- a/src/station.cpp
+++ b/src/station.cpp
@@ -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;
}
}
diff --git a/src/station_base.h b/src/station_base.h
index b832a3c19..af4d206ba 100644
--- a/src/station_base.h
+++ b/src/station_base.h
@@ -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
diff --git a/src/station_cmd.cpp b/src/station_cmd.cpp
index 0d8236ddc..37b553f1a 100644
--- a/src/station_cmd.cpp
+++ b/src/station_cmd.cpp
@@ -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 cargo, StationID next_station_id, uint c
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 cargo, StationID next_station_id, uint c
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 *st, CargoID type, uint amount, SourceT
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 {
diff --git a/src/toolbar_gui.cpp b/src/toolbar_gui.cpp
index 00d844520..45d751dc5 100644
--- a/src/toolbar_gui.cpp
+++ b/src/toolbar_gui.cpp
@@ -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);
}