Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 43 additions & 3 deletions Buildings/Resources/C-Sources/EnergyPlus_24_2_0_Wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define Spawn_declared

#include "EnergyPlus_24_2_0_Wrapper.h"
#include <stdlib.h>

/* *********************************************************
Wrapper functions that connect to the library which
Expand Down Expand Up @@ -69,7 +70,6 @@ void* allocate_Modelica_EnergyPlus_24_2_0(
const double* derivatives_delta,
const size_t nDer){


runPeriod runPer;
runPer.dayOfWeekForStartDay = runPeriod_dayOfWeekForStartDay;
runPer.applyWeekEndHolidayRule = runPeriod_applyWeekEndHolidayRule;
Expand All @@ -78,7 +78,6 @@ void* allocate_Modelica_EnergyPlus_24_2_0(
runPer.use_weatherFileRainIndicators = runPeriod_use_weatherFileRainIndicators;
runPer.use_weatherFileSnowIndicators = runPeriod_use_weatherFileSnowIndicators;


return allocate_Spawn_EnergyPlus_24_2_0(
objectType,
startTime,
Expand Down Expand Up @@ -148,8 +147,49 @@ void exchange_Modelica_EnergyPlus_24_2_0(
y);
}

/* Track freed objects to prevent double-free when OpenModelica calls the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JayHuLBL : If the root cause is indeed multiple calls to free, why not simply setting the pointer to NULL as is recommended at https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/recommendations/memory-management-mem/mem01-c/

This seems much clearer than the rather convoluted tracking mechanism that this pull request would introduce.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwetter
OpenModelica stores the external object pointer in its own runtime and then passes the same (now-freed) address again on the second destructor call.
Setting a local variable to NULL after free has no effect on that external stored value.

destructor multiple times for the same external object instance.
OpenModelica (like Dymola 2019FD01) may call the constructor multiple times
for the same Modelica object and return the same pointer each time.
In that case, the destructor is also called multiple times for that pointer.
We use a simple list to remember which pointers have already been freed. */
static void** EnergyPlus_24_2_0_freed_ptrs = NULL;
static size_t EnergyPlus_24_2_0_freed_count = 0;
static size_t EnergyPlus_24_2_0_freed_capacity = 0;

void free_Modelica_EnergyPlus_24_2_0(void* object){
size_t i;

if (object == NULL) return;

/* Check if this pointer was already freed */
for (i = 0; i < EnergyPlus_24_2_0_freed_count; i++){
if (EnergyPlus_24_2_0_freed_ptrs[i] == object){
/* Already freed - skip to prevent double-free crash */
return;
}
}

/* Record this pointer as freed */
if (EnergyPlus_24_2_0_freed_count >= EnergyPlus_24_2_0_freed_capacity){
void** tmp;
EnergyPlus_24_2_0_freed_capacity = (EnergyPlus_24_2_0_freed_capacity == 0)
? 16 : EnergyPlus_24_2_0_freed_capacity * 2;
tmp = (void**)realloc(EnergyPlus_24_2_0_freed_ptrs,
EnergyPlus_24_2_0_freed_capacity * sizeof(void*));
if (tmp != NULL){
EnergyPlus_24_2_0_freed_ptrs = tmp;
}
else{
/* realloc failed: proceed without tracking (minor risk of double-free) */
EnergyPlus_24_2_0_freed_capacity = EnergyPlus_24_2_0_freed_count;
free_Spawn_EnergyPlus_24_2_0(object);
return;
}
}
EnergyPlus_24_2_0_freed_ptrs[EnergyPlus_24_2_0_freed_count++] = object;

free_Spawn_EnergyPlus_24_2_0(object);
}

#endif
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,67 @@
#include "SpawnFMU.h"

#include <stdlib.h>
#include <stdbool.h>

void free_Spawn_EnergyPlus_24_2_0(void* object){
if ( object != NULL ){
SpawnObject* ptrSpaObj = (SpawnObject*) object;
size_t iBui;
size_t iExcObj;
FMUBuilding* ptrBui = NULL;
size_t foundIdx = 0;
bool found = false;

/* The building may not have been instantiated yet if there was an error during instantiation */
if (ptrSpaObj->bui != NULL){
ptrSpaObj->bui->nExcObj--;
FMUBuildingFree(ptrSpaObj->bui);
/* Search for this SpawnObject in all buildings' exchange arrays by comparing
pointer addresses, without dereferencing ptrSpaObj.
OpenModelica (like Dymola 2019FD01) may call the constructor multiple times
for the same Modelica instance, and the allocator returns the same pointer
each time (see setExchangePointerIfAlreadyInstanciated in SpawnObjectAllocate.c).
In that case, the destructor is also called multiple times for the same pointer.
The first destructor call frees the SpawnObject, making any subsequent
dereference of ptrSpaObj undefined behavior and causing a double-free crash.
By searching the exchange array first (without touching ptrSpaObj's fields),
we can detect and skip repeated destructor calls safely.
Note: If the building was not instantiated due to an error (bui == NULL),
the SpawnObject will not be in any exchange array and will not be freed here.
This is a minor memory leak in the error path which is acceptable since
the simulation will abort anyway. */
for(iBui = 0; iBui < getBuildings_nFMU(); iBui++){
ptrBui = getBuildingsFMU(iBui);
for(iExcObj = 0; iExcObj < ptrBui->nExcObj; iExcObj++){
if (ptrBui->exchange[iExcObj] == object){
foundIdx = iExcObj;
found = true;
break;
}
}
if (found) break;
}
free(ptrSpaObj);

if (found){
/* The SpawnObject is registered in a building's exchange array.
Remove it from the array and perform cleanup. */

/* Remove this SpawnObject from the exchange array by shifting elements */
ptrBui->nExcObj--;
for(iExcObj = foundIdx; iExcObj < ptrBui->nExcObj; iExcObj++){
ptrBui->exchange[iExcObj] = ptrBui->exchange[iExcObj + 1];
}

/* Free the building if no more SpawnObjects reference it */
FMUBuildingFree(ptrBui);

/* Free the SpawnObject itself */
free(ptrSpaObj);
}
/* else: The SpawnObject was not found in any building's exchange array.
This happens when the destructor is called a second time for the same pointer
(double-destructor scenario) - the object was already removed from the
exchange array and freed in the first destructor call. Skip to avoid
a double-free crash.
It can also happen if there was an error during instantiation before
AddSpawnObjectToBuilding was called (bui == NULL). In that case the
SpawnObject is not freed, which is a minor memory leak in an error path
where the simulation would abort anyway. */
}
}
}
Loading