mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2025-02-19 22:16:15 +00:00
Code Issues Projects Releases Packages Wiki Activity

Planner: copy deco state before passing it to worker thread

Browse source 
The planner has a computeVariations() function that can be run
in a worker thread. The code was not thread safe: a deco_state
object allocated on the stack of the caller was passed down to
the worker thread. It's well possible that the object would go
out of scope before the thread run.

Therefore, when running in the background, copy the object first
and free it in the worker thread.

Side note: Qt makes proper memory management again as difficult
as possible: You can't pass a std::unique_ptr<> to QtConcurrent::run,
because move-only objects are not supported. Not very friendly!

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2019-10-17 22:56:40 +02:00 • committed by bstoeger
parent f7c8d65add
commit 769403a4b2
2 changed files with 14 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
qt-models/diveplannermodel.cpp
View file

@ -919,7 +919,10 @@ void DivePlannerPointsModel::createTemporaryPlan()
cloneDiveplan(&diveplan, plan_copy); cloneDiveplan(&diveplan, plan_copy);
unlock_planner(); unlock_planner();
#ifdef VARIATIONS_IN_BACKGROUND #ifdef VARIATIONS_IN_BACKGROUND
QtConcurrent::run(this, &DivePlannerPointsModel::computeVariations, plan_copy, &plan_deco_state); // Since we're calling computeVariations asynchronously and plan_deco_state is allocated
// on the stack, it must be copied and freed by the worker-thread.
struct deco_state *plan_deco_state_copy = new deco_state(plan_deco_state);
QtConcurrent::run(this, &DivePlannerPointsModel::computeVariationsFreeDeco, plan_copy, plan_deco_state_copy);
#else #else
computeVariations(plan_copy, &plan_deco_state); computeVariations(plan_copy, &plan_deco_state);
#endif #endif
@ -1004,7 +1007,13 @@ int DivePlannerPointsModel::analyzeVariations(struct decostop *min, struct decos
return (leftsum + rightsum) / 2; return (leftsum + rightsum) / 2;
} }
void DivePlannerPointsModel::computeVariations(struct diveplan *original_plan, struct deco_state *previos_ds) void DivePlannerPointsModel::computeVariationsFreeDeco(struct diveplan *original_plan, struct deco_state *previous_ds)
{
computeVariations(original_plan, previous_ds);
delete previous_ds;
}
void DivePlannerPointsModel::computeVariations(struct diveplan *original_plan, const struct deco_state *previous_ds)
{ {
// bool oldRecalc = setRecalc(false); // bool oldRecalc = setRecalc(false);
@ -1014,7 +1023,7 @@ void DivePlannerPointsModel::computeVariations(struct diveplan *original_plan, s
struct deco_state *cache = NULL, *save = NULL; struct deco_state *cache = NULL, *save = NULL;
struct diveplan plan_copy; struct diveplan plan_copy;
struct divedatapoint *last_segment; struct divedatapoint *last_segment;
struct deco_state ds = *previos_ds; struct deco_state ds = *previous_ds;
if (!original_plan) if (!original_plan)
return; return;

3
qt-models/diveplannermodel.h
View file

@ -117,7 +117,8 @@ private:
void createPlan(bool replanCopy); void createPlan(bool replanCopy);
struct diveplan diveplan; struct diveplan diveplan;
struct divedatapoint *cloneDiveplan(struct diveplan *plan_src, struct diveplan *plan_copy); struct divedatapoint *cloneDiveplan(struct diveplan *plan_src, struct diveplan *plan_copy);
void computeVariations(struct diveplan *diveplan, struct deco_state *ds); void computeVariations(struct diveplan *diveplan, const struct deco_state *ds);
void computeVariationsFreeDeco(struct diveplan *diveplan, struct deco_state *ds);
int analyzeVariations(struct decostop *min, struct decostop *mid, struct decostop *max, const char *unit); int analyzeVariations(struct decostop *min, struct decostop *mid, struct decostop *max, const char *unit);
Mode mode; Mode mode;
bool recalc; bool recalc;