diff options
author | rubidium <rubidium@openttd.org> | 2007-08-05 21:20:55 +0000 |
---|---|---|
committer | rubidium <rubidium@openttd.org> | 2007-08-05 21:20:55 +0000 |
commit | 83e1fdcb0167c36729889511f7b1165038044c88 (patch) | |
tree | bfe043c51f23052fc39f35769a2ede1e66998148 | |
parent | ab5fa3add2f5f2feeb8f48127f6ee5ab69d42f65 (diff) | |
download | openttd-83e1fdcb0167c36729889511f7b1165038044c88.tar.xz |
(svn r10799) -Fix: only calling QuickFree and not the destructor on pool cleanups might cause memory leaks due to the way C++ works.
-rw-r--r-- | src/depot.cpp | 2 | ||||
-rw-r--r-- | src/group.h | 2 | ||||
-rw-r--r-- | src/group_cmd.cpp | 7 | ||||
-rw-r--r-- | src/oldpool.cpp | 2 | ||||
-rw-r--r-- | src/oldpool.h | 39 | ||||
-rw-r--r-- | src/signs.cpp | 7 | ||||
-rw-r--r-- | src/signs.h | 2 | ||||
-rw-r--r-- | src/station.cpp | 13 | ||||
-rw-r--r-- | src/station.h | 2 | ||||
-rw-r--r-- | src/town.h | 2 | ||||
-rw-r--r-- | src/town_cmd.cpp | 10 | ||||
-rw-r--r-- | src/vehicle.cpp | 12 | ||||
-rw-r--r-- | src/vehicle.h | 2 | ||||
-rw-r--r-- | src/waypoint.cpp | 11 | ||||
-rw-r--r-- | src/waypoint.h | 2 |
15 files changed, 48 insertions, 67 deletions
diff --git a/src/depot.cpp b/src/depot.cpp index 63c6629e1..2f6852bcc 100644 --- a/src/depot.cpp +++ b/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); diff --git a/src/group.h b/src/group.h index b4303e70c..e5c10a59f 100644 --- a/src/group.h +++ b/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; }; diff --git a/src/group_cmd.cpp b/src/group_cmd.cpp index b8e68fbbf..3ac42e2fd 100644 --- a/src/group_cmd.cpp +++ b/src/group_cmd.cpp @@ -50,13 +50,8 @@ 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 diff --git a/src/oldpool.cpp b/src/oldpool.cpp index 3cc5c8391..df4731777 100644 --- a/src/oldpool.cpp +++ b/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); diff --git a/src/oldpool.h b/src/oldpool.h index 38a373474..caeabccab 100644 --- a/src/oldpool.h +++ b/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()) { - t->QuickFree(); - } + delete t; } } @@ -157,15 +164,6 @@ struct PoolItem { } /** - * 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 @@ 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(); + } }; diff --git a/src/signs.cpp b/src/signs.cpp index 8e05f2c01..f8ad95f45 100644 --- a/src/signs.cpp +++ b/src/signs.cpp @@ -28,13 +28,8 @@ Sign::Sign(StringID string) Sign::~Sign() { - this->QuickFree(); - this->str = STR_NULL; -} - -void Sign::QuickFree() -{ DeleteName(this->str); + this->str = STR_NULL; } /** diff --git a/src/signs.h b/src/signs.h index d33578807..05af404eb 100644 --- a/src/signs.h +++ b/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 { diff --git a/src/station.cpp b/src/station.cpp index 4633f1d18..150719e8f 100644 --- a/src/station.cpp +++ b/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 diff --git a/src/station.h b/src/station.h index b6e3596be..866375a42 100644 --- a/src/station.h +++ b/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; diff --git a/src/town.h b/src/town.h index bda85ff78..6d5707146 100644 --- a/src/town.h +++ b/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 { diff --git a/src/town_cmd.cpp b/src/town_cmd.cpp index 4fa9a8c85..48b20ffc7 100644 --- a/src/town_cmd.cpp +++ b/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; diff --git a/src/vehicle.cpp b/src/vehicle.cpp index c845a00b0..78b857c35 100644 --- a/src/vehicle.cpp +++ b/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. diff --git a/src/vehicle.h b/src/vehicle.h index 743987763..63f4203ee 100644 --- a/src/vehicle.h +++ b/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(); diff --git a/src/waypoint.cpp b/src/waypoint.cpp index 425c7a503..9847c9870 100644 --- a/src/waypoint.cpp +++ b/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 diff --git a/src/waypoint.h b/src/waypoint.h index af24b1906..4926d1313 100644 --- a/src/waypoint.h +++ b/src/waypoint.h @@ -30,8 +30,6 @@ struct Waypoint : PoolItem<Waypoint, WaypointID, &_Waypoint_pool> { Waypoint(TileIndex tile = 0); ~Waypoint(); - void QuickFree(); - bool IsValid() const; }; |