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

Catch Throw in Series Ctor/Dtor #709

Merged
merged 2 commits into from
Mar 27, 2020
Merged

Catch Throw in Series Ctor/Dtor #709

merged 2 commits into from
Mar 27, 2020

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Mar 27, 2020

Destructors must not throw, which would abort e.g. if flush() reports a late problem in ~Series or a backend's ~...IOHandler.
Constructors can throw, but we must be able to catch this.

Typical problem: read-only opening of non-existent files throws in Ctor as it flushes all reads.

@ax3l ax3l requested review from franzpoeschel and guj March 27, 2020 02:07
@ax3l ax3l changed the title Test: Catch Throw in ~Series Test: Catch Throw in Series Dtor Mar 27, 2020
@ax3l ax3l changed the title Test: Catch Throw in Series Dtor Catch Throw in Series Dtor Mar 27, 2020
Destructors must not throw, which would abort. Test this.
@ax3l ax3l force-pushed the fix-seriesCatch branch from f8c23ab to 3f524d0 Compare March 27, 2020 02:12
@ax3l ax3l changed the title Catch Throw in Series Dtor Catch Throw in Series Ctor/Dtor Mar 27, 2020
@guj
Copy link
Contributor

guj commented Mar 27, 2020

this does not not necessarily catch the exception from. ~ADIOS2IOHandler(){ flush();}

rather may have caught "fileBased output can not be written with no iterations"
from Series::flushFileBased() first.

@ax3l ax3l force-pushed the fix-seriesCatch branch from a1fceee to eb997dd Compare March 27, 2020 16:22
@ax3l
Copy link
Member Author

ax3l commented Mar 27, 2020

Thank you Junmin! Totally overlooked these.

and write gracefully to `stderr`.
@ax3l ax3l force-pushed the fix-seriesCatch branch from eb997dd to b73738e Compare March 27, 2020 16:23
@guj
Copy link
Contributor

guj commented Mar 27, 2020

Would it be possible to move the code in *.hpp to *.cpp?

@ax3l
Copy link
Member Author

ax3l commented Mar 27, 2020

For ADIOS2IOHandler.hpp? Yes, good idea.

@ax3l
Copy link
Member Author

ax3l commented Mar 27, 2020

Checked again: we build a defaulted destructor and no member vars in the case ADIOS2 is not enabled, so keeping this in the header is fine here.

@ax3l ax3l merged commit 9256a9b into openPMD:dev Mar 27, 2020
@ax3l ax3l deleted the fix-seriesCatch branch March 27, 2020 21:21
@franzpoeschel franzpoeschel mentioned this pull request Jul 9, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants