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.

Sign up for GitHub

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

Fix PixelCounting exception thrown by OpenGL destruction in Windows. #9257

Merged
merged 4 commits into from
Feb 16, 2022
Merged

Fix PixelCounting exception thrown by OpenGL destruction in Windows. #9257

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
401a9ad
Create global state using exported DLL functions.
tanaya-mankad Feb 1, 2022
36b9d15
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into fix…
tanaya-mankad Feb 1, 2022
6a873b9
Fix .clangformat exception.
tanaya-mankad Feb 11, 2022
7627a7d
Merge branch 'fix-opengl-exception' of https://github.com/bigladder/E…
tanaya-mankad Feb 11, 2022
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
15 changes: 9 additions & 6 deletions src/EnergyPlus/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,24 @@
#include <EnergyPlus/Data/EnergyPlusData.hh>
#include <EnergyPlus/DataStringGlobals.hh>
#include <EnergyPlus/api/EnergyPlusPgm.hh>
#include <EnergyPlus/api/state.h>

int main(int argc, const char *argv[])
{
EnergyPlus::EnergyPlusData state;
EnergyPlus::EnergyPlusData *state = reinterpret_cast<EnergyPlus::EnergyPlusData*>(stateNew());
Copy link
Member

Choose a reason for hiding this comment

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

This is indeed interesting. The elimination of global variable usage certainly cleaned up the possibility of having multiple copies all over the place, but I didn't imagine state itself would have been the culprit in this. I believe you, but will need to do some testing to also convince myself. Since you are calling the state API, you can also just use the EnergyPlusState typedef to round out the whole situation.

// these need to be set early to be used in help and version output messaging
// this was pulled from EnergyPlusPgm.cc and needs to be removed once the release is done
Array1D_int value(8);
std::string datestring; // supposedly returns blank when no date available.
date_and_time(datestring, _, _, value);
if (!datestring.empty()) {
state.dataStrGlobals->CurrentDateTime = fmt::format(" YMD={:4}.{:02}.{:02} {:02}:{:02}", value(1), value(2), value(3), value(5), value(6));
state->dataStrGlobals->CurrentDateTime = fmt::format(" YMD={:4}.{:02}.{:02} {:02}:{:02}", value(1), value(2), value(3), value(5), value(6));
} else {
state.dataStrGlobals->CurrentDateTime = " unknown date/time";
state->dataStrGlobals->CurrentDateTime = " unknown date/time";
}
state.dataStrGlobals->VerStringVar = EnergyPlus::DataStringGlobals::VerString + "," + state.dataStrGlobals->CurrentDateTime;
EnergyPlus::CommandLineInterface::ProcessArgs(state, argc, argv);
return EnergyPlusPgm(state);
state->dataStrGlobals->VerStringVar = EnergyPlus::DataStringGlobals::VerString + "," + state->dataStrGlobals->CurrentDateTime;
EnergyPlus::CommandLineInterface::ProcessArgs(*state, argc, argv);
auto ep_ret = EnergyPlusPgm(*state);
stateDelete(reinterpret_cast<EnergyPlusState>(state));
Copy link
Member

Choose a reason for hiding this comment

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

So is this actually necessary for main() to go through the process of cleaning up state when the next line will terminate the program?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An intentional memory leak is actually still a memory leak. I don't love the stateDelete and casting shenanigans, but I'd prefer at least a comment if we remove them and rely on termination cleanup.
These functions happened to be available to us, but are clearly intended for binding through a C interface. The whole thing could be a lot less awkward if the state manipulation in main() could be moved into the DLL entirely. Or, if we had an RAII EnergyPlusState generator class exported from the DLL.

return ep_ret;
}