Skip to content

Commit

Permalink
Ensure that m_lastFlushSuccessful is always called
Browse files Browse the repository at this point in the history
  • Loading branch information
franzpoeschel committed Aug 9, 2023
1 parent 0f3b0f1 commit d8c9b51
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 35 deletions.
8 changes: 8 additions & 0 deletions include/openPMD/IO/AbstractIOHandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,14 @@ class AbstractIOHandler
Access const m_frontendAccess;
internal::SeriesStatus m_seriesStatus = internal::SeriesStatus::Default;
std::queue<IOTask> m_work;
/**
* This is to avoid that the destructor tries flushing again if an error
* happened. Otherwise, this would lead to confusing error messages.
* Initialized as false, set to true after successful construction.
* If flushing results in an error, set this back to false.
* The destructor will only attempt flushing again if this is true.
*/
bool m_lastFlushSuccessful = false;
}; // AbstractIOHandler

} // namespace openPMD
8 changes: 0 additions & 8 deletions include/openPMD/Series.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,6 @@ namespace internal
* True if a user opts into lazy parsing.
*/
bool m_parseLazily = false;
/**
* This is to avoid that the destructor tries flushing again if an error
* happened. Otherwise, this would lead to confusing error messages.
* Initialized as false, set to true after successful construction.
* If flushing results in an error, set this back to false.
* The destructor will only attempt flushing again if this is true.
*/
bool m_lastFlushSuccessful = false;

/**
* In variable-based encoding, all backends except ADIOS2 can only write
Expand Down
13 changes: 12 additions & 1 deletion src/IO/AbstractIOHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,18 @@ namespace openPMD
std::future<void> AbstractIOHandler::flush(internal::FlushParams const &params)
{
internal::ParsedFlushParams parsedParams{params};
auto future = this->flush(parsedParams);
auto future = [this, &parsedParams]() {
try
{
return this->flush(parsedParams);
}
catch (...)
{
m_lastFlushSuccessful = false;
throw;
}
}();
m_lastFlushSuccessful = true;
json::warnGlobalUnusedOptions(parsedParams.backendConfig);
return future;
}
Expand Down
32 changes: 8 additions & 24 deletions src/Series.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ Given file pattern: ')END"
break;
}
}
series.m_lastFlushSuccessful = true;
IOHandler()->m_lastFlushSuccessful = true;
}

void Series::initDefaults(IterationEncoding ie, bool initAll)
Expand Down Expand Up @@ -711,7 +711,7 @@ std::future<void> Series::flush_impl(
bool flushIOHandler)
{
auto &series = get();
series.m_lastFlushSuccessful = true;
IOHandler()->m_lastFlushSuccessful = true;
try
{
switch (iterationEncoding())
Expand All @@ -736,7 +736,7 @@ std::future<void> Series::flush_impl(
}
catch (...)
{
series.m_lastFlushSuccessful = false;
IOHandler()->m_lastFlushSuccessful = false;
throw;
}
}
Expand Down Expand Up @@ -1892,15 +1892,7 @@ AdvanceStatus Series::advance(
// We cannot call Series::flush now, since the IO handler is still filled
// from calling flush(Group|File)based, but has not been emptied yet
// Do that manually
try
{
IOHandler()->flush(flushParams);
}
catch (...)
{
series.m_lastFlushSuccessful = false;
throw;
}
IOHandler()->flush(flushParams);

return *param.status;
}
Expand Down Expand Up @@ -1966,15 +1958,7 @@ AdvanceStatus Series::advance(AdvanceMode mode)
// We cannot call Series::flush now, since the IO handler is still filled
// from calling flush(Group|File)based, but has not been emptied yet
// Do that manually
try
{
IOHandler()->flush(flushParams);
}
catch (...)
{
series.m_lastFlushSuccessful = false;
throw;
}
IOHandler()->flush(flushParams);

return *param.status;
}
Expand Down Expand Up @@ -2300,10 +2284,10 @@ namespace internal
* `Series` is needlessly flushed a second time. Otherwise, error
* messages can get very confusing.
*/
if (this->m_lastFlushSuccessful && m_writable.IOHandler &&
m_writable.IOHandler->has_value())
Series impl{{this, [](auto const *) {}}};
if (auto IOHandler = impl.IOHandler();
IOHandler && IOHandler->m_lastFlushSuccessful)
{
Series impl{{this, [](auto const *) {}}};
impl.flush();
/*
* In file-based iteration encoding, this must be triggered by
Expand Down
4 changes: 2 additions & 2 deletions src/WriteIterations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ WriteIterations::SharedResources::SharedResources(

WriteIterations::SharedResources::~SharedResources()
{
if (currentlyOpen.has_value() &&
iterations.retrieveSeries().get().m_lastFlushSuccessful)
if (auto IOHandler = iterations.IOHandler(); currentlyOpen.has_value() &&
IOHandler && IOHandler->m_lastFlushSuccessful)
{
auto lastIterationIndex = currentlyOpen.value();
auto &lastIteration = iterations.at(lastIterationIndex);
Expand Down

0 comments on commit d8c9b51

Please sign in to comment.