Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leak Fixes #28

Merged
merged 19 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
31bee3d
feat: add listDelete for linked list
hellkite500 Jun 13, 2024
e099fa7
fix: deallocate memory with listDelete before overwriting with listCopy
hellkite500 Jun 13, 2024
093a8a1
fix: add destructor to deallocate class held pointers
hellkite500 Jun 13, 2024
997a2b7
fix: ensure cleanup of old memory when allocating soil params
hellkite500 Jun 13, 2024
f9f302b
fix: delete memory when deleting front from list
hellkite500 Jun 13, 2024
0d1f493
fix: delete tempory heap allocated variables in bmi_main
hellkite500 Jun 13, 2024
3353322
fix: delete temporary dynamic allocated arrays
hellkite500 Jun 13, 2024
bb5684e
fix: clean up list memory prior to listCopy in main.cxx
hellkite500 Jun 13, 2024
7f63385
fix: cleanup dynamic memory in bmi state during Finalize
hellkite500 Jun 13, 2024
06a3c2e
fix: call Finalize on bmi model
hellkite500 Jun 13, 2024
18580d7
chore: inline comments on things that don't make sense
hellkite500 Jun 13, 2024
1914cd8
fix: free instead of delete for malloc'd mem
hellkite500 Jun 13, 2024
e652bb2
fix: free another set of tempory allocated pointers
hellkite500 Jun 13, 2024
8605e5e
fix: explicitly intialize class pointers to nullptr
hellkite500 Jun 13, 2024
a046a34
chore: change recursive call to loop in listDelete
hellkite500 Jul 10, 2024
025b4c7
chore: remove redundant NULL check
hellkite500 Jul 10, 2024
fb08aea
Ptl add adaptive timestep (#25)
peterlafollette Jun 26, 2024
3fab3e8
fix: remove extra mass balance check at end of bmi_main test
hellkite500 Jul 10, 2024
d52d6f0
chore: remove defensive checks for nullptr
hellkite500 Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/all.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ extern void listReverseOrder(struct wetting_front** head_ref
extern bool listFindLayer(struct wetting_front* link, int num_layers, double *cum_layer_thickness_cm,
int *lives_in_layer, bool *extends_to_bottom_flag);
extern struct wetting_front* listCopy(struct wetting_front* current, struct wetting_front* state_previous=NULL);

extern void listDelete(struct wetting_front* head);



Expand Down
4 changes: 3 additions & 1 deletion include/bmi_lgar.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class NotImplemented : public std::logic_error {

class BmiLGAR : public bmi::Bmi {
public:
BmiLGAR() {
~BmiLGAR();
BmiLGAR():giuh_ordinates(nullptr), giuh_runoff_queue(nullptr) {
this->input_var_names[0] = "precipitation_rate";
this->input_var_names[1] = "potential_evapotranspiration_rate";
this->input_var_names[2] = "soil_temperature_profile";
Expand Down Expand Up @@ -131,6 +132,7 @@ public:
struct model_state* get_model();

private:
void realloc_soil();
struct model_state* state;
static const int input_var_name_count = 3;
static const int output_var_name_count = 15;
Expand Down
59 changes: 55 additions & 4 deletions src/bmi_lgar.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
// default verbosity is set to 'none' other option 'high' or 'low' needs to be specified in the config file
string verbosity="none";

/**
* @brief Delete dynamic arrays allocated in Initialize() and held by this object
*
*/
BmiLGAR::~BmiLGAR(){
if( giuh_ordinates != nullptr ) delete [] giuh_ordinates;
if( giuh_runoff_queue != nullptr ) delete [] giuh_runoff_queue;
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved
}

/* The `head` pointer stores the address in memory of the first member of the linked list containing
all the wetting fronts. The contents of struct wetting_front are defined in "all.h" */
Expand Down Expand Up @@ -48,6 +56,20 @@ Initialize (std::string config_file)

}

/**
* @brief Allocate (or reallocate) storage for soil parameters
*
*/
void BmiLGAR::realloc_soil(){
if(state->lgar_bmi_params.soil_depth_wetting_fronts != nullptr)
delete [] state->lgar_bmi_params.soil_depth_wetting_fronts;
if(state->lgar_bmi_params.soil_moisture_wetting_fronts != nullptr)
delete [] state->lgar_bmi_params.soil_moisture_wetting_fronts;

state->lgar_bmi_params.soil_depth_wetting_fronts = new double[state->lgar_bmi_params.num_wetting_fronts];
state->lgar_bmi_params.soil_moisture_wetting_fronts = new double[state->lgar_bmi_params.num_wetting_fronts];
}
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved

/*
This is the main function calling lgar subroutines for creating, moving, and merging wetting fronts.
Calls to AET and mass balance module are also happening here
Expand Down Expand Up @@ -143,7 +165,10 @@ Update()
std::cerr<<"BMI Update |Timesteps = "<< state->lgar_bmi_params.timesteps<<", Time [h] = "<<this->state->lgar_bmi_params.time_s / 3600.<<", Subcycle = "<< cycle <<" of "<<subcycles<<std::endl;
}

state->state_previous = NULL;
if( state->state_previous != NULL ){
listDelete(state->state_previous);
state->state_previous = NULL;
}
state->state_previous = listCopy(state->head);

// ensure precip and PET are non-negative
Expand Down Expand Up @@ -270,7 +295,10 @@ Update()
listPrint(state->head);
}

state->state_previous = NULL;
if(state->state_previous != NULL ){
listDelete(state->state_previous);
state->state_previous = NULL;
}
state->state_previous = listCopy(state->head);

volin_timestep_cm += volin_subtimestep_cm;
Expand Down Expand Up @@ -422,8 +450,7 @@ Update()
state->lgar_bmi_params.num_wetting_fronts = listLength(state->head);

// allocate new memory based on updated wetting fronts; we could make it conditional i.e. create only if no. of wf are changed
state->lgar_bmi_params.soil_depth_wetting_fronts = new double[state->lgar_bmi_params.num_wetting_fronts];
state->lgar_bmi_params.soil_moisture_wetting_fronts = new double[state->lgar_bmi_params.num_wetting_fronts];
realloc_soil();

// update thickness/depth and soil moisture of wetting fronts (used for state coupling)
struct wetting_front *current = state->head;
Expand Down Expand Up @@ -583,6 +610,30 @@ void BmiLGAR::
Finalize()
{
global_mass_balance();
if( state->head != NULL ) listDelete(state->head);
if( state->state_previous != NULL ) listDelete(state->state_previous);

if( state->soil_properties != nullptr ) delete [] state->soil_properties;

if( state->lgar_bmi_params.soil_depth_wetting_fronts != nullptr ) delete [] state->lgar_bmi_params.soil_depth_wetting_fronts;
if( state->lgar_bmi_params.soil_moisture_wetting_fronts != nullptr) delete [] state->lgar_bmi_params.soil_moisture_wetting_fronts;

if( state->lgar_bmi_params.soil_temperature != nullptr ) delete [] state->lgar_bmi_params.soil_temperature;
if( state->lgar_bmi_params.soil_temperature_z != nullptr) delete [] state->lgar_bmi_params.soil_temperature_z;
if( state->lgar_bmi_params.layer_soil_type != nullptr ) delete [] state->lgar_bmi_params.layer_soil_type;

if( state->lgar_calib_params.theta_e != nullptr ) delete [] state->lgar_calib_params.theta_e;
if( state->lgar_calib_params.theta_r != nullptr ) delete [] state->lgar_calib_params.theta_r;
if( state->lgar_calib_params.vg_n != nullptr ) delete [] state->lgar_calib_params.vg_n;
if( state->lgar_calib_params.vg_alpha != nullptr ) delete [] state->lgar_calib_params.vg_alpha;
if( state->lgar_calib_params.Ksat != nullptr ) delete [] state->lgar_calib_params.Ksat;

if( state->lgar_bmi_params.layer_thickness_cm != nullptr ) delete [] state->lgar_bmi_params.layer_thickness_cm;
if( state->lgar_bmi_params.cum_layer_thickness_cm != nullptr ) delete [] state->lgar_bmi_params.cum_layer_thickness_cm;
if( state->lgar_bmi_params.giuh_ordinates != nullptr ) delete [] state->lgar_bmi_params.giuh_ordinates;
if( state->lgar_bmi_params.frozen_factor != nullptr ) delete [] state->lgar_bmi_params.frozen_factor;
if( state->lgar_bmi_input_params != nullptr ) delete state->lgar_bmi_input_params;
if( state != nullptr ) delete state;
}


Expand Down
4 changes: 3 additions & 1 deletion src/bmi_main_lgar.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,15 @@ int main(int argc, char *argv[])
// write layers data to file
fprintf(outlayer_fptr,"# Timestep = %d, %s \n", i, time[i].c_str());
write_state(outlayer_fptr, model_state.get_model()->head);
delete [] soil_moisture_wetting_front;
delete [] soil_thickness_wetting_front;
}

}

// do final mass balance
model_state.global_mass_balance();
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved

model_state.Finalize();
if (outdata_fptr) {
fclose(outdata_fptr);
fclose(outlayer_fptr);
Expand Down
11 changes: 9 additions & 2 deletions src/lgar.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,7 @@ extern void InitFromConfigFile(string config_file, struct model_state *state)
}
}
else {
//NJF FIXME these arrays should be allocated based on num_cells_temp...
state->lgar_bmi_params.soil_temperature = new double[1]();
state->lgar_bmi_params.soil_temperature_z = new double[1]();
state->lgar_bmi_params.num_cells_temp = 1;
Expand Down Expand Up @@ -1117,6 +1118,8 @@ extern void lgar_move_wetting_fronts(double timestep_h, double *volin_cm, int wf
double theta_below = 0.0;

new_mass += layer_thickness * (theta - theta_below);
//NJF theta_below is always 0, so all delta_thetas are always 0...
//does this really need a dynamic array in this case???
delta_thetas[k] = theta_below;
delta_thickness[k] = layer_thickness;
}
Expand All @@ -1134,7 +1137,9 @@ extern void lgar_move_wetting_fronts(double timestep_h, double *volin_cm, int wf
double theta_new = lgar_theta_mass_balance(layer_num, soil_num, psi_cm, new_mass, prior_mass, AET_demand_cm,
delta_thetas, delta_thickness, soil_type, soil_properties);
actual_ET_demand = *AET_demand_cm;

//done with delta_thetas and delta_thickness, cleanup memory
free(delta_thetas);
free(delta_thickness);
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved
current->theta = fmax(theta_r, fmin(theta_new, theta_e));

double Se = calc_Se_from_theta(current->theta,theta_e,theta_r);
Expand Down Expand Up @@ -1276,7 +1281,9 @@ extern void lgar_move_wetting_fronts(double timestep_h, double *volin_cm, int wf
double theta_new = lgar_theta_mass_balance(layer_num, soil_num, psi_cm, new_mass, prior_mass, AET_demand_cm,
delta_thetas, delta_thickness, soil_type, soil_properties);
actual_ET_demand = *AET_demand_cm;

//done with delta_thetas and delta_thickness, cleanup memory
free(delta_thetas);
free(delta_thickness);
current->theta = fmax(theta_r, fmin(theta_new, theta_e));

}
Expand Down
16 changes: 15 additions & 1 deletion src/linked_list.cxx
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@
//___________________________________________________________


/*###########################################################*/
/* listDelete() - deletes memory allocated to a linked list */
/* This function must be called on any list to deallocate */
/* the dynamic memory used in creating and manipultating the */
/* list. (added by NJF) */
/*###########################################################*/
extern void listDelete(struct wetting_front* head)
{
if( head != NULL ){
struct wetting_front *next = head->next;
free( head );
listDelete(next);
}
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved
}

/*#########################################################*/
/* listPrint() - prints a linked list to screen */
Expand Down Expand Up @@ -240,7 +254,7 @@ extern struct wetting_front* listDeleteFront(int front_num, struct wetting_front
previous = current->next;

}

if( current != NULL ) free( current );
current = previous;

while(previous != NULL) { // decrement all front numbers
Expand Down
2 changes: 2 additions & 0 deletions src/main.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ for(time_step_num=0;time_step_num<num_time_steps;time_step_num++)
printf(" ************ time step %d ******** \n",time_step_num);
}

if( state_previous != NULL ) listDelete(state_previous);
state_previous = NULL;
state_previous = listCopy(head);
hellkite500 marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -542,6 +543,7 @@ for(time_step_num=0;time_step_num<num_time_steps;time_step_num++)
lgar_create_surfacial_front(&ponded_depth_cm, &volin_timestep_cm, dry_depth, theta1, soil_type_by_layer, soil_properties, cum_layer_thickness_cm, nint, time_step_h);

//listPrint();
if( state_previous != NULL ) listDelete(state_previous);
state_previous = NULL;
state_previous = listCopy(head);

Expand Down
Loading