IMPALA-9571: Fix Impala crash from unexpected boost filesystem_error exception
The no-exception signature of boost::filesystem::remove_all() has a
longstanding bug where it can throw an exception even for the
no-exceptions interface. Boost recently added the "noexcept" specifier
to the no-exception signature of remove_all(). When a "noexcept"
function throws an exception, it causes the program to call std::terminate()
which aborts the program. This makes the try/catch block around
remove_all() useless.
As a point fix, this switches FilesystemUtil to use the remove_all()
signature that can throw exceptions. This code already had a try/catch
block around remove_all(), so now it just uses that to get the error
code.
Newer versions of boost (1.63+) fix the underlying problem, so this
code can be removed when we upgrade boost.
Testing:
- Ran core tests
- Ran FilesystemUtil backend test
Change-Id: I23016f37401604fc41fc99c0a056439a31333e3f
Reviewed-on: http://gerrit.cloudera.org:8080/15640
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Reviewed-by: Csaba Ringhofer <csringhofer@cloudera.com>
(cherry picked from commit 76061331781e67b90d7750abadfaf8b013dc64a5)
diff --git a/be/src/util/filesystem-util.cc b/be/src/util/filesystem-util.cc
index ea0a10a..b4fce78 100644
--- a/be/src/util/filesystem-util.cc
+++ b/be/src/util/filesystem-util.cc
@@ -87,10 +87,16 @@
// the check for existence above and the removal here. If the directory is removed in
// this window, we may get "no_such_file_or_directory" error which is fine.
//
- // There is a bug in boost library (as of version 1.6) which may lead to unexpected
- // exceptions even though we are using the no-exceptions interface. See IMPALA-2846.
+ // There is a bug in boost library (as of version 1.61) which may lead to unexpected
+ // exceptions from the no-exceptions interface. Even worse, the no-exceptions
+ // interface is marked as noexcept, so the unexpected exception will cause the
+ // program to immediately call std::terminate(). This renders the try/catch block
+ // useless. For now, this uses the interface that can throw exceptions and
+ // gets the errorcode from the exception. See IMPALA-2846 + IMPALA-9571.
+ // TODO: Newer boost has a fix for this bug, so this can be switched back after
+ // upgrading boost.
try {
- filesystem::remove_all(directory, errcode);
+ filesystem::remove_all(directory);
} catch (filesystem::filesystem_error& e) {
errcode = e.code();
}
@@ -111,10 +117,11 @@
Status FileSystemUtil::RemovePaths(const vector<string>& directories) {
for (int i = 0; i < directories.size(); ++i) {
error_code errcode;
- // There is a bug in boost library (as of version 1.6) which may lead to unexpected
- // exceptions even though we are using the no-exceptions interface. See IMPALA-2846.
+ // This uses the boost remove_all() interface that can throw exceptions to avoid a
+ // bug in the implementation of the no-exceptions remove_all(). See comment in
+ // RemoveAndCreateDirectory() for more details.
try {
- filesystem::remove_all(directories[i], errcode);
+ filesystem::remove_all(directories[i]);
} catch (filesystem::filesystem_error& e) {
errcode = e.code();
}
