wowgetoffyourcellphone/0ad
1
0
Fork 0
forked from 0ad/0ad
Code Pull Requests Activity

Resolve Loader memory leak from 1f1642bfe3.

Browse Source 
Use C++11 featured shared_ptr to avoid conditional deletion in multiple
locations.
Drops C compatibility(?) of Loader.h.

Differential Revision: https://code.wildfiregames.com/D2331
Tested on: clang 8.0.1., Jenkins
Comments on IRC #wfg 2005-05-07
Refs 77d3c5d0b5, 5e3b0f06ec, f19d8dafee.

This was SVN commit r23019.
This commit is contained in:
elexis 2019-09-30 08:49:00 +00:00
parent 218399bf3d
commit 1679510bb8
3 changed files with 21 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
source/ps/Loader.cpp
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2009 Wildfire Games.
/* Copyright (C) 2019 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -62,18 +62,17 @@ struct LoadRequest
// member documentation is in LDR_Register (avoid duplication).
LoadFunc func;
void* param;
// MemFun_t<T> instance
std::shared_ptr<void> param;
// Translatable string shown to the player.
CStrW description;
// rationale for storing as CStrW here:
// - needs to be WCS because it's user-visible and will be translated.
// - don't just store a pointer - the caller's string may be volatile.
// - the module interface must work in C, so we get/set as wchar_t*.
int estimated_duration_ms;
// LDR_Register gets these as parameters; pack everything together.
LoadRequest(LoadFunc func_, void* param_, const wchar_t* desc_, int ms_)
LoadRequest(LoadFunc func_, std::shared_ptr<void> param_, const wchar_t* desc_, int ms_)
: func(func_), param(param_), description(desc_),
estimated_duration_ms(ms_)
{
@ -109,13 +108,13 @@ void LDR_BeginRegistering()
// register a task (later processed in FIFO order).
// <func>: function that will perform the actual work; see LoadFunc.
// <param>: (optional) parameter/persistent state; must be freed by func.
// <param>: (optional) parameter/persistent state.
// <description>: user-visible description of the current task, e.g.
// "Loading Textures".
// <estimated_duration_ms>: used to calculate progress, and when checking
// whether there is enough of the time budget left to process this task
// (reduces timeslice overruns, making the main loop more responsive).
void LDR_Register(LoadFunc func, void* param, const wchar_t* description,
void LDR_Register(LoadFunc func, std::shared_ptr<void> param, const wchar_t* description,
int estimated_duration_ms)
{
ENSURE(state == REGISTERING); // must be called between LDR_(Begin|End)Register

8
source/ps/Loader.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2009 Wildfire Games.
/* Copyright (C) 2019 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -116,17 +116,17 @@ extern void LDR_BeginRegistering();
// != 0, or it's treated as "finished")
// - on failure, return a negative error code or 'warning' (see above);
// LDR_ProgressiveLoad will abort immediately and return that.
typedef int (*LoadFunc)(void* param, double time_left);
typedef int (*LoadFunc)(std::shared_ptr<void> param, double time_left);
// register a task (later processed in FIFO order).
// <func>: function that will perform the actual work; see LoadFunc.
// <param>: (optional) parameter/persistent state; must be freed by func.
// <param>: (optional) parameter/persistent state.
// <description>: user-visible description of the current task, e.g.
// "Loading Textures".
// <estimated_duration_ms>: used to calculate progress, and when checking
// whether there is enough of the time budget left to process this task
// (reduces timeslice overruns, making the main loop more responsive).
extern void LDR_Register(LoadFunc func, void* param, const wchar_t* description,
extern void LDR_Register(LoadFunc func, std::shared_ptr<void> param, const wchar_t* description,
int estimated_duration_ms);

36
source/ps/LoaderThunks.h
View File

@ -1,4 +1,4 @@
/* Copyright (C) 2009 Wildfire Games.
/* Copyright (C) 2019 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -18,15 +18,6 @@
#ifndef INCLUDED_LOADERTHUNKS
#define INCLUDED_LOADERTHUNKS
// rationale for allocating MemFun_t dynamically:
// need to store class pointer, function, and argument for each registered
// function; single static storage isn't possible. we don't want to break
// C compat in the Loader.h interface, so we can't have it take care of this.
// that leaves dynamic alloc or reserving some static storage freed when
// load registration begins. the former is slower and requires checking
// the thunked function's return value (because we mustn't free MemFun_t
// if the function times out), but is simpler.
// VC7 warns if T::*func is not aligned to its size (4..16 bytes on IA-32).
// this is a bug, since sizeof(void*) would be enough. MS says it won't
// be fixed: see http://www.dotnet247.com/247reference/msgs/1/7782.aspx
@ -54,21 +45,16 @@ public:
: this_(this__), func(func_) {}
};
template<class T> static int MemFunThunk(void* param, double UNUSED(time_left))
template<class T> static int MemFunThunk(std::shared_ptr<void> param, double UNUSED(time_left))
{
MemFun_t<T>* const mf = (MemFun_t<T>*)param;
int ret = (mf->this_->*mf->func)();
if(!ldr_was_interrupted(ret))
delete mf;
return ret;
MemFun_t<T>* const mf = static_cast<MemFun_t<T>*>(param.get());
return (mf->this_->*mf->func)();
}
template<class T> void RegMemFun(T* this_, int(T::*func)(void),
const wchar_t* description, int estimated_duration_ms)
{
void* param = new MemFun_t<T>(this_, func);
LDR_Register(MemFunThunk<T>, param, description, estimated_duration_ms);
LDR_Register(MemFunThunk<T>, std::make_shared<MemFun_t<T>>(this_, func), description, estimated_duration_ms);
}
@ -86,20 +72,16 @@ public:
: this_(this__), func(func_), arg(arg_) {}
};
template<class T, class Arg> static int MemFun1Thunk(void* param, double UNUSED(time_left))
template<class T, class Arg> static int MemFun1Thunk(shared_ptr<void> param, double UNUSED(time_left))
{
MemFun1_t<T, Arg>* const mf = (MemFun1_t<T, Arg>*)param;
int ret = (mf->this_->*mf->func)(mf->arg);
if(!ldr_was_interrupted(ret))
delete mf;
return ret;
MemFun1_t<T, Arg>* const mf = static_cast<MemFun1_t<T, Arg>*>(param.get());
return (mf->this_->*mf->func)(mf->arg);
}
template<class T, class Arg> void RegMemFun1(T* this_, int(T::*func)(Arg), Arg arg,
const wchar_t* description, int estimated_duration_ms)
{
void* param = new MemFun1_t<T, Arg>(this_, func, arg);
LDR_Register(MemFun1Thunk<T, Arg>, param, description, estimated_duration_ms);
LDR_Register(MemFun1Thunk<T, Arg>, std::make_shared<MemFun1_t<T, Arg> >(this_, func, arg), description, estimated_duration_ms);
}
#endif // INCLUDED_LOADERTHUNKS