(svn r10799) -Fix: only calling QuickFree and not the destructor on pool cleanups might cause memory leaks due to the way C++ works.
--- a/src/depot.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/depot.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -37,6 +37,8 @@
*/
Depot::~Depot()
{
+ if (CleaningPool()) return;
+
/* Clear the depot from all order-lists */
RemoveOrderFromAllVehicles(OT_GOTO_DEPOT, this->index);
--- a/src/group.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/group.h Sun Aug 05 21:20:55 2007 +0000
@@ -29,8 +29,6 @@
Group(StringID str = STR_NULL);
virtual ~Group();
- void QuickFree();
-
bool IsValid() const;
};
--- a/src/group_cmd.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/group_cmd.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -50,15 +50,10 @@
Group::~Group()
{
- this->QuickFree();
+ DeleteName(this->string_id);
this->string_id = STR_NULL;
}
-void Group::QuickFree()
-{
- DeleteName(this->string_id);
-}
-
bool Group::IsValid() const
{
return this->string_id != STR_NULL;
--- a/src/oldpool.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/oldpool.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -18,6 +18,7 @@
DEBUG(misc, 4, "[Pool] (%s) cleaning pool..", this->name);
+ this->cleaning_pool = true;
/* Free all blocks */
for (i = 0; i < this->current_blocks; i++) {
if (this->clean_block_proc != NULL) {
@@ -25,6 +26,7 @@
}
free(this->blocks[i]);
}
+ this->cleaning_pool = false;
/* Free the block itself */
free(this->blocks);
--- a/src/oldpool.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/oldpool.h Sun Aug 05 21:20:55 2007 +0000
@@ -25,7 +25,7 @@
OldMemoryPoolNewBlock *new_block_proc, OldMemoryPoolCleanBlock *clean_block_proc) :
name(name), max_blocks(max_blocks), block_size_bits(block_size_bits),
new_block_proc(new_block_proc), clean_block_proc(clean_block_proc), current_blocks(0),
- total_items(0), item_size(item_size), first_free_index(0), blocks(NULL) {}
+ total_items(0), cleaning_pool(false), item_size(item_size), first_free_index(0), blocks(NULL) {}
const char* name; ///< Name of the pool (just for debugging)
@@ -40,6 +40,7 @@
uint current_blocks; ///< How many blocks we have in our pool
uint total_items; ///< How many items we now have in this pool
+ bool cleaning_pool; ///< Are we currently cleaning the pool?
public:
const uint item_size; ///< How many bytes one block is
uint first_free_index; ///< The index of the first free pool item in this pool
@@ -84,6 +85,15 @@
{
return this->name;
}
+
+ /**
+ * Is the pool in the cleaning phase?
+ * @return true if it is
+ */
+ inline bool CleaningPool() const
+ {
+ return this->cleaning_pool;
+ }
};
template <typename T>
@@ -121,7 +131,6 @@
/**
* Generic function to free a new block in a pool.
- * This function uses QuickFree that is intended to only free memory that would be lost if the pool is freed.
* @param start_item the first item that needs to be cleaned
* @param end_item the last item that needs to be cleaned
*/
@@ -130,9 +139,7 @@
{
for (uint i = start_item; i <= end_item; i++) {
T *t = Tpool->Get(i);
- if (t->IsValid()) {
- t->QuickFree();
- }
+ delete t;
}
}
@@ -157,15 +164,6 @@
}
/**
- * Called on each object when the pool is being destroyed, so one
- * can free allocated memory without the need for freeing for
- * example orders.
- */
- virtual void QuickFree()
- {
- }
-
- /**
* An overriden version of new that allocates memory on the pool.
* @param size the size of the variable (unused)
* @return the memory that is 'allocated'
@@ -241,7 +239,7 @@
* Allocate a pool item; possibly allocate a new block in the pool.
* @return the allocated pool item (or NULL when the pool is full).
*/
- static T *AllocateRaw()
+ static inline T *AllocateRaw()
{
return AllocateRaw(Tpool->first_free_index);
}
@@ -251,7 +249,7 @@
* @param first the first pool item to start searching
* @return the allocated pool item (or NULL when the pool is full).
*/
- static T *AllocateRaw(uint &first)
+ static inline T *AllocateRaw(uint &first)
{
uint last_minus_one = Tpool->GetSize() - 1;
@@ -271,6 +269,15 @@
return NULL;
}
+
+ /**
+ * Are we cleaning this pool?
+ * @return true if we are
+ */
+ static inline bool CleaningPool()
+ {
+ return Tpool->CleaningPool();
+ }
};
--- a/src/signs.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/signs.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -28,15 +28,10 @@
Sign::~Sign()
{
- this->QuickFree();
+ DeleteName(this->str);
this->str = STR_NULL;
}
-void Sign::QuickFree()
-{
- DeleteName(this->str);
-}
-
/**
*
* Update the coordinate of one sign
--- a/src/signs.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/signs.h Sun Aug 05 21:20:55 2007 +0000
@@ -27,8 +27,6 @@
~Sign();
bool IsValid() const { return this->str != STR_NULL; }
-
- void QuickFree();
};
enum {
--- a/src/station.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/station.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -64,6 +64,11 @@
{
DEBUG(station, cDebugCtorLevel, "I-%3d", index);
+ DeleteName(this->string_id);
+ free(this->speclist);
+
+ if (CleaningPool()) return;
+
MarkDirty();
RebuildStationLists();
InvalidateWindowClasses(WC_STATION_LIST);
@@ -81,14 +86,6 @@
for (CargoID c = 0; c < NUM_CARGO; c++) {
goods[c].cargo.Truncate(0);
}
-
- this->QuickFree();
-}
-
-void Station::QuickFree()
-{
- DeleteName(this->string_id);
- free(this->speclist);
}
/** Called when new facility is built on the station. If it is the first facility
--- a/src/station.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/station.h Sun Aug 05 21:20:55 2007 +0000
@@ -159,8 +159,6 @@
Station(TileIndex tile = 0);
virtual ~Station();
- void QuickFree();
-
void AddFacility(byte new_facility_bit, TileIndex facil_xy);
void MarkDirty() const;
void MarkTilesDirty(bool cargo_change) const;
--- a/src/town.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/town.h Sun Aug 05 21:20:55 2007 +0000
@@ -160,8 +160,6 @@
~Town();
bool IsValid() const { return this->xy != 0; }
-
- void QuickFree();
};
struct HouseSpec {
--- a/src/town_cmd.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/town_cmd.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -53,6 +53,10 @@
Town::~Town()
{
+ DeleteName(this->townnametype);
+
+ if (CleaningPool()) return;
+
Industry *i;
/* Delete town authority window
@@ -87,15 +91,9 @@
MarkWholeScreenDirty();
- this->QuickFree();
this->xy = 0;
}
-void Town::QuickFree()
-{
- DeleteName(this->townnametype);
-}
-
// Local
static int _grow_town_result;
--- a/src/vehicle.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/vehicle.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -564,6 +564,8 @@
void Vehicle::PreDestructor()
{
+ if (CleaningPool()) return;
+
if (IsValidStationID(this->last_station_visited)) {
GetStation(this->last_station_visited)->loading_vehicles.remove(this);
@@ -579,7 +581,6 @@
if (this->IsPrimaryVehicle()) DecreaseGroupNumVehicle(this->group_id);
}
- this->QuickFree();
if (this->type == VEH_ROAD) ClearSlot(this);
if (this->type != VEH_TRAIN || (this->type == VEH_TRAIN && (IsFrontEngine(this) || IsFreeWagon(this)))) {
@@ -599,6 +600,10 @@
Vehicle::~Vehicle()
{
+ DeleteName(this->string_id);
+
+ if (CleaningPool()) return;
+
UpdateVehiclePosHash(this, INVALID_COORD, 0);
this->next_hash = NULL;
this->next_new_hash = NULL;
@@ -608,11 +613,6 @@
new (this) InvalidVehicle();
}
-void Vehicle::QuickFree()
-{
- DeleteName(this->string_id);
-}
-
/**
* Deletes all vehicles in a chain.
* @param v The first vehicle in the chain.
--- a/src/vehicle.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/vehicle.h Sun Aug 05 21:20:55 2007 +0000
@@ -352,8 +352,6 @@
/** We want to 'destruct' the right class. */
virtual ~Vehicle();
- void QuickFree();
-
void BeginLoading();
void LeaveStation();
--- a/src/waypoint.cpp Sun Aug 05 17:43:04 2007 +0000
+++ b/src/waypoint.cpp Sun Aug 05 21:20:55 2007 +0000
@@ -404,17 +404,14 @@
Waypoint::~Waypoint()
{
+ if (this->string != STR_NULL) DeleteName(this->string);
+
+ if (CleaningPool()) return;
+
RemoveOrderFromAllVehicles(OT_GOTO_WAYPOINT, this->index);
RedrawWaypointSign(this);
this->xy = 0;
-
- this->QuickFree();
-}
-
-void Waypoint::QuickFree()
-{
- if (this->string != STR_NULL) DeleteName(this->string);
}
bool Waypoint::IsValid() const
--- a/src/waypoint.h Sun Aug 05 17:43:04 2007 +0000
+++ b/src/waypoint.h Sun Aug 05 21:20:55 2007 +0000
@@ -30,8 +30,6 @@
Waypoint(TileIndex tile = 0);
~Waypoint();
- void QuickFree();
-
bool IsValid() const;
};