Skip to content

Commit f76c87a

Browse files
committed
utl: ensure exception-safe file closure in stream and file handlers
Addresses an issue where utl::OutStreamHandler could cause a crash (std::terminate) if a file flush or rename failed during destruction (e.g., due to ENOSPC). - Added explicit close() methods to OutStreamHandler, InStreamHandler, and FileHandler to allow users to handle closure errors explicitly. - Updated destructors to be exception-safe by catching potential errors during implicit closure. - Added diagnostic logging to std::cerr in destructors to report closure failures that would otherwise be suppressed. Fixes #9252 Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
1 parent 0c68cb3 commit f76c87a

File tree

2 files changed

+53
-7
lines changed

2 files changed

+53
-7
lines changed

src/utl/include/utl/ScopedTemporaryFile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class OutStreamHandler
4949
OutStreamHandler(const char* filename, bool binary = false);
5050
~OutStreamHandler();
5151
std::ostream& getStream();
52+
void close();
5253

5354
private:
5455
std::string filename_;
@@ -66,6 +67,7 @@ class InStreamHandler
6667
InStreamHandler(const char* filename, bool binary = false);
6768
~InStreamHandler();
6869
std::istream& getStream();
70+
void close();
6971

7072
private:
7173
std::string filename_;
@@ -82,6 +84,7 @@ class FileHandler
8284
FileHandler(const char* filename, bool binary = false);
8385
~FileHandler();
8486
FILE* getFile();
87+
void close();
8588

8689
private:
8790
std::string filename_;

src/utl/src/ScopedTemporaryFile.cpp

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <exception>
1010
#include <filesystem>
1111
#include <ios>
12+
#include <iostream>
1213
#include <istream>
1314
#include <memory>
1415
#include <ostream>
@@ -95,6 +96,19 @@ OutStreamHandler::OutStreamHandler(const char* filename, bool binary)
9596
}
9697

9798
OutStreamHandler::~OutStreamHandler()
99+
{
100+
try {
101+
close();
102+
} catch (const std::exception& e) {
103+
std::cerr << "Error: close for " << filename_ << " failed: " << e.what()
104+
<< "\n";
105+
} catch (...) {
106+
std::cerr << "Error: close for " << filename_
107+
<< " failed with unknown error\n";
108+
}
109+
}
110+
111+
void OutStreamHandler::close()
98112
{
99113
if (stream_) {
100114
boost::iostreams::close(*buf_);
@@ -105,9 +119,9 @@ OutStreamHandler::~OutStreamHandler()
105119
if (os_.is_open()) {
106120
// Any pending output sequence is written to the file.
107121
os_.close();
122+
// If filename_ exists it will be overwritten
123+
fs::rename(tmp_filename_, filename_);
108124
}
109-
// If filename_ exists it will be overwritten
110-
fs::rename(tmp_filename_, filename_);
111125
}
112126

113127
std::ostream& OutStreamHandler::getStream()
@@ -144,6 +158,19 @@ InStreamHandler::InStreamHandler(const char* filename, bool binary)
144158
}
145159

146160
InStreamHandler::~InStreamHandler()
161+
{
162+
try {
163+
close();
164+
} catch (const std::exception& e) {
165+
std::cerr << "Error: close for " << filename_ << " failed: " << e.what()
166+
<< "\n";
167+
} catch (...) {
168+
std::cerr << "Error: close for " << filename_
169+
<< " failed with unknown error\n";
170+
}
171+
}
172+
173+
void InStreamHandler::close()
147174
{
148175
if (stream_) {
149176
boost::iostreams::close(*buf_);
@@ -152,7 +179,6 @@ InStreamHandler::~InStreamHandler()
152179
}
153180

154181
if (is_.is_open()) {
155-
// Any pending output sequence is written to the file.
156182
is_.close();
157183
}
158184
}
@@ -177,13 +203,30 @@ FileHandler::FileHandler(const char* filename, bool binary)
177203
}
178204

179205
FileHandler::~FileHandler()
206+
{
207+
try {
208+
close();
209+
} catch (const std::exception& e) {
210+
std::cerr << "Error: close for " << filename_ << " failed: " << e.what()
211+
<< "\n";
212+
} catch (...) {
213+
std::cerr << "Error: close for " << filename_
214+
<< " failed with unknown error\n";
215+
}
216+
}
217+
218+
void FileHandler::close()
180219
{
181220
if (file_) {
182-
// Any unwritten buffered data are flushed to the OS.
183-
std::fclose(file_);
221+
const bool failed = (std::fclose(file_) != 0);
222+
file_ = nullptr;
223+
if (failed) {
224+
throw std::runtime_error("fclose failed for " + tmp_filename_ + ": "
225+
+ strerror(errno));
226+
}
227+
// If filename_ exists it will be overwritten.
228+
fs::rename(tmp_filename_, filename_);
184229
}
185-
// If filename_ exists it will be overwritten.
186-
fs::rename(tmp_filename_, filename_);
187230
}
188231

189232
FILE* FileHandler::getFile()

0 commit comments

Comments
 (0)