Skip to content

Commit da3795c

Browse files
daverigbytrondn
authored andcommitted
MB-45378: Avoid crash with boost::fs global object
On MSVC 2017 if platform is linked statically, we see a crash in ~EpBucketImpl when removing the dbPath temp directory: msvcp140.dll!_guard_dispatch_icall_nop() Line 55 Unknown msvcp140.dll!std::codecvt<unsigned short,char,_Mbstatet>::in(_Mbstatet & _State, const char * _First1, const char * _Last1, const char * & _Mid1, unsigned short * _First2, unsigned short * _Last2, unsigned short * & _Mid2) Line 728 C++ memcached_testapp.exe!boost::filesystem::path_traits::convert(wchar_t const *,wchar_t const *,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > &,class std::codecvt<wchar_t,char,struct _Mbstatet> const &) C++ memcached_testapp.exe!boost::filesystem::path_traits::convert(char const *,char const *,class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > &,class std::codecvt<wchar_t,char,struct _Mbstatet> const &) C++ > memcached_testapp.exe!boost::filesystem::path_traits::convert(const char * from, const char * from_end, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & to) Line 1006 C++ memcached_testapp.exe!boost::filesystem::path_traits::dispatch<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & c, std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > & to) Line 257 C++ memcached_testapp.exe!boost::filesystem::path::path<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & source, void * __formal) Line 168 C++ memcached_testapp.exe!cb::io::makeExtendedLengthPath(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & path) Line 46 C++ memcached_testapp.exe!cb::io::rmrf(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & path) Line 221 C++ memcached_testapp.exe!EpBucketImpl::~EpBucketImpl() Line 214 C++ [External Code] memcached_testapp.exe!McdEnvironment::~McdEnvironment() Line 388 C++ [External Code] memcached_testapp.exe!testing::internal::Delete<testing::Environment>(testing::Environment * x) Line 342 C++ [External Code] memcached_testapp.exe!testing::internal::ForEach<std::vector<testing::Environment *,std::allocator<testing::Environment *> >,void (__cdecl*)(testing::Environment *)>(const std::vector<testing::Environment *,std::allocator<testing::Environment *> > & c, void(*)(testing::Environment *) functor) Line 295 C++ memcached_testapp.exe!testing::internal::UnitTestImpl::~UnitTestImpl() Line 5033 C++ [External Code] memcached_testapp.exe!testing::UnitTest::~UnitTest() Line 4979 C++ [External Code] This appears to be a known issue[1] with boost::filesystem, due to it using a static global for locale conversion - making any calls to boost::filesystem::path to/from string after this locale object has been deleted can crash the program. This is happening inside cb::io::rmrf when it manipulates paths to delete all files under the given dbPath. Avoid this issue by changing dbPath to boost::fs::path - meaning we already have a object which has performed locale conversion at shutdown, then replacing the call to cb::io::rmrf with boost::fs::remove_all(), which doesn't perform any additional locale manipulation. [1]: https://www.boost.org/doc/libs/1_76_0/libs/log/doc/html/log/rationale/why_crash_on_term.html Change-Id: I1783e669392b9abedabfe59d0249e429a51fe826 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/152073 Tested-by: Trond Norbye <[email protected]> Reviewed-by: Trond Norbye <[email protected]>
1 parent 8e163b2 commit da3795c

File tree

2 files changed

+11
-10
lines changed

2 files changed

+11
-10
lines changed

tests/testapp/testapp_environment.cc

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111
#include "testapp_environment.h"
1212
#include "memcached_audit_events.h"
13+
#include <boost/filesystem.hpp>
1314
#include <folly/portability/Stdlib.h>
1415
#include <platform/dirutils.h>
1516
#include <platform/strerror.h>
@@ -204,15 +205,15 @@ class EpBucketImpl : public TestBucketImpl {
204205
: TestBucketImpl(extraConfig), dbPath(cb::io::mkdtemp("mc_testapp")) {
205206
// Cleanup any files from a previous run still on disk.
206207
try {
207-
cb::io::rmrf(dbPath);
208+
cb::io::rmrf(dbPath.string());
208209
} catch (...) { /* nothing exists */
209210
}
210211
}
211212

212213
~EpBucketImpl() override {
213214
// Cleanup any files created.
214215
try {
215-
cb::io::rmrf(dbPath);
216+
boost::filesystem::remove_all(dbPath);
216217
} catch (...) { /* nothing exists */
217218
}
218219
}
@@ -226,12 +227,12 @@ class EpBucketImpl : public TestBucketImpl {
226227
void createBucket(const std::string& name,
227228
const std::string& config,
228229
MemcachedConnection& conn) override {
229-
const auto dbdir = cb::io::sanitizePath(dbPath + "/" + name);
230+
const auto dbdir = cb::io::sanitizePath((dbPath / name).string());
230231
if (cb::io::isDirectory(dbdir)) {
231232
cb::io::rmrf(dbdir);
232233
}
233234

234-
std::string settings = "dbname=" + dbPath + "/" + name;
235+
std::string settings = "dbname=" + (dbPath / name).generic_string();
235236
if (!config.empty()) {
236237
settings += ";" + config;
237238
}
@@ -261,13 +262,13 @@ class EpBucketImpl : public TestBucketImpl {
261262
void setUpBucket(const std::string& name,
262263
const std::string& config,
263264
MemcachedConnection& conn) override {
264-
const auto dbdir = cb::io::sanitizePath(dbPath + "/" + name);
265+
const auto dbdir = cb::io::sanitizePath((dbPath / name).string());
265266
if (cb::io::isDirectory(dbdir) &&
266267
bucketCreateMode == BucketCreateMode::Clean) {
267268
cb::io::rmrf(dbdir);
268269
}
269270

270-
std::string settings = "dbname=" + dbPath + "/" + name;
271+
std::string settings = "dbname=" + (dbPath / name).generic_string();
271272
// Increase bucket quota from 100MB to 200MB as there are some
272273
// testapp tests requiring more than the default.
273274
settings += ";max_size=200000000";
@@ -356,7 +357,7 @@ class EpBucketImpl : public TestBucketImpl {
356357
}
357358

358359
/// Directory for any database files.
359-
const std::string dbPath;
360+
const boost::filesystem::path dbPath;
360361
};
361362

362363
McdEnvironment::McdEnvironment(bool manageSSL_,
@@ -482,6 +483,6 @@ void McdEnvironment::rewriteRbacFile() {
482483
}
483484
}
484485

485-
const std::string& McdEnvironment::getDbPath() const {
486-
return static_cast<EpBucketImpl*>(testBucket.get())->dbPath;
486+
std::string McdEnvironment::getDbPath() const {
487+
return static_cast<EpBucketImpl*>(testBucket.get())->dbPath.string();
487488
}

tests/testapp/testapp_environment.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ class McdEnvironment : public ::testing::Environment {
214214
/**
215215
* @return The dbPath of a persistent bucket (throws if not persistent)
216216
*/
217-
const std::string& getDbPath() const;
217+
std::string getDbPath() const;
218218

219219
private:
220220
void SetupAuditFile();

0 commit comments

Comments
 (0)