(svn r10799) -Fix: only calling QuickFree and not the destructor on pool cleanups might cause memory leaks due to the way C++ works.

src/depot.cpp

@@ -37,6 +37,8 @@ Depot *GetDepotByTile(TileIndex tile)
  */
 Depot::~Depot()
 {
+	if (CleaningPool()) return;
+
 	/* Clear the depot from all order-lists */
 	RemoveOrderFromAllVehicles(OT_GOTO_DEPOT, this->index);
 

src/group.h

@@ -29,8 +29,6 @@ struct Group : PoolItem<Group, GroupID, &_Group_pool> {
 	Group(StringID str = STR_NULL);
 	virtual ~Group();
 
-	void QuickFree();
-
 	bool IsValid() const;
 };
 

src/group_cmd.cpp

@@ -49,14 +49,9 @@ Group::Group(StringID str)
 }
 
 Group::~Group()
-{
-	this->QuickFree();
-	this->string_id = STR_NULL;
-}
-
-void Group::QuickFree()
 {
 	DeleteName(this->string_id);
+	this->string_id = STR_NULL;
 }
 
 bool Group::IsValid() const

src/oldpool.cpp

@@ -18,6 +18,7 @@ void OldMemoryPoolBase::CleanPool()
 
 	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 @@ void OldMemoryPoolBase::CleanPool()
 		}
 		free(this->blocks[i]);
 	}
+	this->cleaning_pool = false;
 
 	/* Free the block itself */
 	free(this->blocks);

src/oldpool.h

@@ -25,7 +25,7 @@ protected:
 				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 @@ protected:
 	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 @@ public:
 	{
 		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 @@ static void PoolNewBlock(uint start_item)
 
 /**
  * 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 @@ static void PoolCleanBlock(uint start_item, uint end_item)
 {
 	for (uint i = start_item; i <= end_item; i++) {
 		T *t = Tpool->Get(i);
-		if (t->IsValid()) {
+		delete t;
-			t->QuickFree();
-		}
 	}
 }
 
@@ -156,15 +163,6 @@ struct PoolItem {
 		if (this->index < Tpool->first_free_index) Tpool->first_free_index = this->index;
 	}
 
-	/**
-	 * 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)
@@ -241,7 +239,7 @@ protected:
 	 * 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 @@ protected:
 	 * @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 @@ protected:
 
 		return NULL;
 	}
+
+	/**
+	 * Are we cleaning this pool?
+	 * @return true if we are
+	 */
+	static inline bool CleaningPool()
+	{
+		return Tpool->CleaningPool();
+	}
 };
 
 

src/signs.cpp

@@ -27,14 +27,9 @@ Sign::Sign(StringID string)
 }
 
 Sign::~Sign()
-{
-	this->QuickFree();
-	this->str = STR_NULL;
-}
-
-void Sign::QuickFree()
 {
 	DeleteName(this->str);
+	this->str = STR_NULL;
 }
 
 /**

src/signs.h

@@ -27,8 +27,6 @@ struct Sign : PoolItem<Sign, SignID, &_Sign_pool> {
 	~Sign();
 
 	bool IsValid() const { return this->str != STR_NULL; }
-
-	void QuickFree();
 };
 
 enum {

src/station.cpp

@@ -64,6 +64,11 @@ Station::~Station()
 {
 	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 @@ Station::~Station()
 	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

src/station.h

@@ -159,8 +159,6 @@ struct Station : PoolItem<Station, StationID, &_Station_pool> {
 	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;

src/town.h

@@ -160,8 +160,6 @@ struct Town : PoolItem<Town, TownID, &_Town_pool> {
 	~Town();
 
 	bool IsValid() const { return this->xy != 0; }
-
-	void QuickFree();
 };
 
 struct HouseSpec {

src/town_cmd.cpp

@@ -53,6 +53,10 @@ Town::Town(TileIndex tile)
 
 Town::~Town()
 {
+	DeleteName(this->townnametype);
+
+	if (CleaningPool()) return;
+
 	Industry *i;
 
 	/* Delete town authority window
@@ -87,15 +91,9 @@ Town::~Town()
 
 	MarkWholeScreenDirty();
 
-	this->QuickFree();
 	this->xy = 0;
 }
 
-void Town::QuickFree()
-{
-	DeleteName(this->townnametype);
-}
-
 // Local
 static int _grow_town_result;
 

src/vehicle.cpp

@@ -564,6 +564,8 @@ bool IsEngineCountable(const Vehicle *v)
 
 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 @@ void Vehicle::PreDestructor()
 		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 @@ void Vehicle::PreDestructor()
 
 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 @@ Vehicle::~Vehicle()
 	new (this) InvalidVehicle();
 }
 
-void Vehicle::QuickFree()
-{
-	DeleteName(this->string_id);
-}
-
 /**
  * Deletes all vehicles in a chain.
  * @param v The first vehicle in the chain.

src/vehicle.h

@@ -352,8 +352,6 @@ struct Vehicle : PoolItem<Vehicle, VehicleID, &_Vehicle_pool> {
 	/** We want to 'destruct' the right class. */
 	virtual ~Vehicle();
 
-	void QuickFree();
-
 	void BeginLoading();
 	void LeaveStation();
 

src/waypoint.cpp

@@ -404,17 +404,14 @@ Waypoint::Waypoint(TileIndex tile)
 
 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

src/waypoint.h

@@ -30,8 +30,6 @@ struct Waypoint : PoolItem<Waypoint, WaypointID, &_Waypoint_pool> {
 	Waypoint(TileIndex tile = 0);
 	~Waypoint();
 
-	void QuickFree();
-
 	bool IsValid() const;
 };