Mercurial > hg > openttd
changeset 7413:571c87eed06c draft
(svn r10799) -Fix: only calling QuickFree and not the destructor on pool cleanups might cause memory leaks due to the way C++ works.
author | rubidium <rubidium@openttd.org> |
---|---|
date | Sun, 05 Aug 2007 21:20:55 +0000 |
parents | 6682e85ffa07 |
children | 2496ac7d0086 |
files | src/depot.cpp src/group.h src/group_cmd.cpp src/oldpool.cpp src/oldpool.h src/signs.cpp src/signs.h src/station.cpp src/station.h src/town.h src/town_cmd.cpp src/vehicle.cpp src/vehicle.h src/waypoint.cpp src/waypoint.h |
diffstat | 15 files changed, 48 insertions(+), 67 deletions(-) [+] |
line wrap: on
line diff
--- a/src/depot.cpp +++ b/src/depot.cpp @@ -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 +++ b/src/group.h @@ -29,8 +29,6 @@ Group(StringID str = STR_NULL); virtual ~Group(); - void QuickFree(); - bool IsValid() const; };
--- a/src/group_cmd.cpp +++ b/src/group_cmd.cpp @@ -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 +++ b/src/oldpool.cpp @@ -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 +++ b/src/oldpool.h @@ -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 +++ b/src/signs.cpp @@ -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 +++ b/src/signs.h @@ -27,8 +27,6 @@ ~Sign(); bool IsValid() const { return this->str != STR_NULL; } - - void QuickFree(); }; enum {
--- a/src/station.cpp +++ b/src/station.cpp @@ -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 +++ b/src/station.h @@ -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 +++ b/src/town.h @@ -160,8 +160,6 @@ ~Town(); bool IsValid() const { return this->xy != 0; } - - void QuickFree(); }; struct HouseSpec {
--- a/src/town_cmd.cpp +++ b/src/town_cmd.cpp @@ -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 +++ b/src/vehicle.cpp @@ -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 +++ b/src/vehicle.h @@ -352,8 +352,6 @@ /** We want to 'destruct' the right class. */ virtual ~Vehicle(); - void QuickFree(); - void BeginLoading(); void LeaveStation();
--- a/src/waypoint.cpp +++ b/src/waypoint.cpp @@ -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