summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Jamroot.jam1
-rw-r--r--netfs/daemon/modeCheck.cpp1
-rw-r--r--netfs/fuse/fuseApp.cpp17
-rw-r--r--netfs/fuse/fuseApp.h2
-rw-r--r--netfs/fuse/fuseAppBase.h1
-rw-r--r--netfs/fuse/netfs.cpp6
-rw-r--r--netfs/lib/Jamfile.jam3
-rw-r--r--netfs/lib/entCache.cpp3
-rw-r--r--netfs/lib/entCache.h5
-rw-r--r--netfs/lib/entCache.impl.h13
-rw-r--r--netfs/unittests/Jamfile.jam5
-rw-r--r--netfs/unittests/leak-suppressions.txt3
-rw-r--r--netfs/unittests/mockDaemon.cpp3
-rw-r--r--netfs/unittests/mockDaemon.h2
-rw-r--r--netfs/unittests/mockFuse.cpp5
-rw-r--r--netfs/unittests/testCore.cpp2
-rw-r--r--netfs/unittests/testEdgeCases.cpp12
-rw-r--r--netfs/unittests/testFuse.cpp59
-rw-r--r--netfs/unittests/testGlacier.cpp2
-rw-r--r--netfs/unittests/testLib.cpp19
-rw-r--r--netfs/unittests/thread-suppressions.txt5
21 files changed, 119 insertions, 50 deletions
diff --git a/Jamroot.jam b/Jamroot.jam
index 61c7d8b..e78971c 100644
--- a/Jamroot.jam
+++ b/Jamroot.jam
@@ -30,6 +30,7 @@ project
<toolset>tidy:<xcheckxx>hicpp-vararg
<toolset>tidy:<xcheckxx>hicpp-signed-bitwise
<toolset>tidy:<xcheckxx>hicpp-no-array-decay
+ <toolset>tidy:<xcheckxx>hicpp-named-parameter
<toolset>tidy:<checkxx>performance-*
<toolset>tidy:<exclude>daemonConfig.h
<toolset>tidy:<exclude>slicer-daemonConfig.cpp
diff --git a/netfs/daemon/modeCheck.cpp b/netfs/daemon/modeCheck.cpp
index 177d9b8..d63163a 100644
--- a/netfs/daemon/modeCheck.cpp
+++ b/netfs/daemon/modeCheck.cpp
@@ -17,6 +17,7 @@ ModeCheck::AssertReadParent(const std::filesystem::path & p) const
}
void
+// NOLINTNEXTLINE(misc-no-recursion)
ModeCheck::AssertRead(const std::filesystem::path & p) const
{
if (p != root) {
diff --git a/netfs/fuse/fuseApp.cpp b/netfs/fuse/fuseApp.cpp
index 04602aa..b6d59b1 100644
--- a/netfs/fuse/fuseApp.cpp
+++ b/netfs/fuse/fuseApp.cpp
@@ -159,8 +159,8 @@ NetFS::FuseApp::opt_parse(void * data, const char * arg, int, struct fuse_args *
void
NetFS::FuseApp::connectSession()
{
+ Lock(_lock);
if (!sessionOpened && ic->getDefaultRouter()) {
- Lock(_lock);
auto router = Ice::checkedCast<Glacier2::RouterPrx>(ic->getDefaultRouter());
session = router->createSession("", "");
if (int acmTimeout = router->getACMTimeout() > 0) {
@@ -174,8 +174,8 @@ NetFS::FuseApp::connectSession()
void
NetFS::FuseApp::connectToService()
{
+ Lock(_lock);
if (!service) {
- Lock(_lock);
auto proxyAddr = fcr->ServiceIdentity;
for (const auto & ep : fcr->Endpoints) {
proxyAddr += ":" + ep;
@@ -188,10 +188,10 @@ NetFS::FuseApp::connectToService()
}
void
-NetFS::FuseApp::connectToVolume()
+NetFS::FuseApp::connectToVolume(bool force)
{
- if (!volume) {
- Lock(_lock);
+ Lock(_lock);
+ if (force || !volume) {
volume = service->connect(fcr->ExportName, fcr->AuthToken);
if (!volume) {
throw std::runtime_error("Invalid filesystem proxy");
@@ -260,7 +260,7 @@ NetFS::FuseApp::onError(const std::exception & e) noexcept
verifyConnection();
connectSession();
connectToService();
- connectToVolume();
+ connectToVolume(false);
connectHandles();
return 0;
}
@@ -270,8 +270,7 @@ NetFS::FuseApp::onError(const std::exception & e) noexcept
}
if (dynamic_cast<const Ice::RequestFailedException *>(&e)) {
try {
- volume = nullptr;
- connectToVolume();
+ connectToVolume(true);
connectHandles();
return 0;
}
@@ -290,7 +289,7 @@ NetFS::FuseApp::beforeOperation()
{
connectSession();
connectToService();
- connectToVolume();
+ connectToVolume(false);
}
NetFS::ReqEnv
diff --git a/netfs/fuse/fuseApp.h b/netfs/fuse/fuseApp.h
index b905b23..9eafb7f 100644
--- a/netfs/fuse/fuseApp.h
+++ b/netfs/fuse/fuseApp.h
@@ -38,7 +38,7 @@ namespace NetFS {
protected:
void connectSession();
virtual void connectToService();
- void connectToVolume();
+ void connectToVolume(bool force);
void connectHandles();
void verifyConnection();
diff --git a/netfs/fuse/fuseAppBase.h b/netfs/fuse/fuseAppBase.h
index 44f3c81..4fdfb37 100644
--- a/netfs/fuse/fuseAppBase.h
+++ b/netfs/fuse/fuseAppBase.h
@@ -140,6 +140,7 @@ private:
{
if constexpr (!std::is_same<BFunc, IFunc>::value) {
return [](Args... a) {
+ SharedLock(fuseApp->_lock);
return (fuseApp->*bfunc)(a...);
};
}
diff --git a/netfs/fuse/netfs.cpp b/netfs/fuse/netfs.cpp
index 9df3a75..77530ac 100644
--- a/netfs/fuse/netfs.cpp
+++ b/netfs/fuse/netfs.cpp
@@ -1,4 +1,5 @@
#include "fuseApp.h"
+#include <c++11Helpers.h>
#include <syslog.h>
class FuseImpl : public fuse_args, public NetFS::FuseApp {
@@ -14,16 +15,13 @@ public:
{
openlog("netfs", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER);
}
- FuseImpl(const FuseImpl &) = delete;
- FuseImpl(FuseImpl &&) = delete;
~FuseImpl() override
{
closelog();
}
- void operator=(const FuseImpl &) = delete;
- void operator=(FuseImpl &&) = delete;
+ SPECIAL_MEMBERS_DELETE(FuseImpl);
struct fuse_context *
fuse_get_context() override
diff --git a/netfs/lib/Jamfile.jam b/netfs/lib/Jamfile.jam
index e1199a8..b3a3285 100644
--- a/netfs/lib/Jamfile.jam
+++ b/netfs/lib/Jamfile.jam
@@ -5,7 +5,8 @@ lib netfs-common :
<link>static
<use>..//adhocutil/<link>shared
<implicit-dependency>../ice//netfs-api/<link>shared
- <cxxflags>-fPIC
+ <toolset>gcc:<cxxflags>-fPIC
+ <toolset>clang:<cxxflags>-fPIC
: :
<include>.
<library>../ice//netfs-api/<link>shared
diff --git a/netfs/lib/entCache.cpp b/netfs/lib/entCache.cpp
index 6f6d6eb..a2ea685 100644
--- a/netfs/lib/entCache.cpp
+++ b/netfs/lib/entCache.cpp
@@ -9,7 +9,6 @@ const int BUFLEN = 8196;
void
UserEntCache::fillCache() const noexcept
{
- Lock(lock);
setpwent();
idcache->clear();
std::array<char, BUFLEN> buf {};
@@ -27,7 +26,6 @@ GroupEntCache::GroupEntCache(EntryResolverPtr<User> u) : users(std::move(u)) { }
void
GroupEntCache::fillCache() const noexcept
{
- Lock(lock);
setgrent();
std::array<char, BUFLEN> buf {};
idcache->clear();
@@ -40,6 +38,7 @@ GroupEntCache::fillCache() const noexcept
g->members.insert(ent->id);
}
}
+ g->members.insert(g->id);
idcache->insert(std::move(g));
}
endgrent();
diff --git a/netfs/lib/entCache.h b/netfs/lib/entCache.h
index e394b05..7c8567a 100644
--- a/netfs/lib/entCache.h
+++ b/netfs/lib/entCache.h
@@ -4,6 +4,7 @@
#include "entries.h"
#include "entryResolver.h"
#include <c++11Helpers.h>
+#include <memory>
#include <shared_mutex>
template<class entry_t> class EntCache : public EntryResolver<entry_t> {
@@ -28,8 +29,8 @@ public:
protected:
virtual void fillCache() const noexcept = 0;
- template<class key_t>[[nodiscard]] entry_ptr getEntryInternal(const key_t &) const noexcept;
- template<class key_t>[[nodiscard]] entry_ptr getEntryNoFill(const key_t &) const noexcept;
+ template<class key_t> [[nodiscard]] entry_ptr getEntryInternal(const key_t &) const noexcept;
+ template<class key_t> [[nodiscard]] entry_ptr getEntryNoFill(const key_t &) const noexcept;
class Ids;
std::unique_ptr<Ids> idcache;
diff --git a/netfs/lib/entCache.impl.h b/netfs/lib/entCache.impl.h
index 5e6289e..96bf4eb 100644
--- a/netfs/lib/entCache.impl.h
+++ b/netfs/lib/entCache.impl.h
@@ -8,6 +8,7 @@
#include <boost/multi_index_container.hpp>
#include <grp.h>
#include <lockHelpers.h>
+#include <mutex>
#include <pwd.h>
#include <type_traits>
@@ -28,12 +29,19 @@ template<class key_t>
typename EntCache<entry_t>::entry_ptr
EntCache<entry_t>::getEntryInternal(const key_t & key) const noexcept
{
- if (fillTime + 60 > time(nullptr)) {
+ SharedScopeLock(lock) {
+ if (fillTime + 60 > time(nullptr)) {
+ if (auto ent = getEntryNoFill<key_t>(key)) {
+ return ent;
+ }
+ }
+ }
+ ScopeLock(lock) {
+ fillCache();
if (auto ent = getEntryNoFill<key_t>(key)) {
return ent;
}
}
- fillCache();
return getEntryNoFill<key_t>(key);
}
@@ -42,7 +50,6 @@ template<class key_t>
typename EntCache<entry_t>::entry_ptr
EntCache<entry_t>::getEntryNoFill(const key_t & key) const noexcept
{
- SharedLock(lock);
auto & collection = idcache->template get<key_t>();
auto i = collection.find(key);
if (i != collection.end()) {
diff --git a/netfs/unittests/Jamfile.jam b/netfs/unittests/Jamfile.jam
index 8d9efc3..fc9d00b 100644
--- a/netfs/unittests/Jamfile.jam
+++ b/netfs/unittests/Jamfile.jam
@@ -1,11 +1,16 @@
lib boost_utf : : <name>boost_unit_test_framework ;
+path-constant leak-san-suppressions : leak-suppressions.txt ;
+path-constant thread-san-suppressions : thread-suppressions.txt ;
+
project
: requirements
<toolset>tidy:<xcheckxx>misc-non-private-member-variables-in-classes
<toolset>tidy:<xcheckxx>hicpp-special-member-functions
<toolset>tidy:<xcheckxx>hicpp-explicit-conversions
<toolset>tidy:<xcheckxx>hicpp-member-init
+ <leak-sanitizer>on:<testing.launcher>LSAN_OPTIONS=suppressions=$(leak-san-suppressions)
+ <thread-sanitizer>on:<testing.launcher>TSAN_OPTIONS=suppressions=$(thread-san-suppressions)
;
path-constant me : . ;
diff --git a/netfs/unittests/leak-suppressions.txt b/netfs/unittests/leak-suppressions.txt
new file mode 100644
index 0000000..42eae90
--- /dev/null
+++ b/netfs/unittests/leak-suppressions.txt
@@ -0,0 +1,3 @@
+# OpenLDAP leaks when calling getpwent/getgrent
+leak:ber_memalloc_x
+leak:ber_memcalloc_x
diff --git a/netfs/unittests/mockDaemon.cpp b/netfs/unittests/mockDaemon.cpp
index 0cb3172..1ccd6b7 100644
--- a/netfs/unittests/mockDaemon.cpp
+++ b/netfs/unittests/mockDaemon.cpp
@@ -36,7 +36,6 @@ MockDaemonHost::stop()
{
if (daemon) {
daemon->stop();
- delete daemon;
}
if (ic) {
ic->destroy();
@@ -51,7 +50,7 @@ MockDaemonHost::start()
id.properties->parseCommandLineOptions("", params);
ic = Ice::initialize(id);
ic->getProperties()->setProperty("NetFSD.HostNameOverride", "unittest");
- daemon = new MockDaemon(testEndpoint);
+ daemon = std::make_unique<MockDaemon>(testEndpoint);
daemon->start("NetFSDaemonAdapter", ic, {});
}
diff --git a/netfs/unittests/mockDaemon.h b/netfs/unittests/mockDaemon.h
index fb129c4..8bc6097 100644
--- a/netfs/unittests/mockDaemon.h
+++ b/netfs/unittests/mockDaemon.h
@@ -33,7 +33,7 @@ private:
const std::string testEndpoint;
Ice::StringSeq params;
- NetFSDaemon * daemon;
+ std::unique_ptr<NetFSDaemon> daemon;
};
#endif
diff --git a/netfs/unittests/mockFuse.cpp b/netfs/unittests/mockFuse.cpp
index 554a62d..b283d7b 100644
--- a/netfs/unittests/mockFuse.cpp
+++ b/netfs/unittests/mockFuse.cpp
@@ -37,7 +37,10 @@ FuseMock::connectToService()
void
FuseMock::vlogf(int, const char * fmt, va_list args) const noexcept
{
- BOOST_TEST_MESSAGE(vstringf(fmt, args));
+ static std::mutex btm;
+ ScopeLock(btm) {
+ BOOST_TEST_MESSAGE(vstringf(fmt, args));
+ }
}
FuseMockHost::FuseMockHost(std::string ep, const Ice::StringSeq & a) :
diff --git a/netfs/unittests/testCore.cpp b/netfs/unittests/testCore.cpp
index cdbb958..1e68be5 100644
--- a/netfs/unittests/testCore.cpp
+++ b/netfs/unittests/testCore.cpp
@@ -653,7 +653,7 @@ BOOST_AUTO_TEST_CASE(interval_map_works_how_i_think)
auto check = [&](off_t o, size_t s, int c) {
BOOST_TEST_CONTEXT("offset: " << o << ", size: " << s << ", count: " << c) {
const auto ol = map.equal_range(r(o, s));
- BOOST_TEST_CONTEXT("range: " << ol.first->first.lower() << " to " << ol.first->first.upper()) {
+ BOOST_TEST_CONTEXT("range start: " << ol.first->first.lower()) {
BOOST_REQUIRE_EQUAL(std::distance(ol.first, ol.second), c);
}
}
diff --git a/netfs/unittests/testEdgeCases.cpp b/netfs/unittests/testEdgeCases.cpp
index 01eb6b7..17dd706 100644
--- a/netfs/unittests/testEdgeCases.cpp
+++ b/netfs/unittests/testEdgeCases.cpp
@@ -89,20 +89,23 @@ BOOST_AUTO_TEST_CASE(manyThreads)
MockDaemonHost daemon(testEndpoint, {"--NetFSD.ConfigPath=" + (rootDir / "defaultDaemon.xml").string()});
FuseMockHost fuse(testEndpoint, {(rootDir / "defaultFuse.xml:testvol").string(), (rootDir / "test").string()});
- bool running = true;
+ std::atomic<bool> running {true};
std::vector<std::thread> ths;
ths.reserve(20);
- std::atomic_uint success = 0;
+ std::atomic<unsigned int> success {0}, failure {0};
for (int x = 0; x < 20; x++) {
ths.emplace_back([&] {
struct statvfs s {
};
- while (running && success < 1600) {
+ while (running) {
if (fuse.fuse->statfs("/", &s) == 0) {
success++;
}
+ else {
+ failure++;
+ }
+ usleep(10000);
}
- usleep(10000);
});
}
@@ -116,4 +119,5 @@ BOOST_AUTO_TEST_CASE(manyThreads)
}
BOOST_CHECK_GT(success, 800);
+ BOOST_CHECK_EQUAL(failure, 0);
}
diff --git a/netfs/unittests/testFuse.cpp b/netfs/unittests/testFuse.cpp
index 0ad9335..b0603b0 100644
--- a/netfs/unittests/testFuse.cpp
+++ b/netfs/unittests/testFuse.cpp
@@ -1,12 +1,14 @@
#define BOOST_TEST_MODULE TestNetFSFuse
#include "mockDaemon.h"
#include <boost/test/unit_test.hpp>
+#include <c++11Helpers.h>
#include <cache.impl.h>
#include <definedDirs.h>
#include <filesystem>
#include <fuse.h>
#include <fuseApp.h>
#include <fuseMappersImpl.h>
+#include <lockHelpers.h>
#include <ostream>
#include <thread>
@@ -18,35 +20,46 @@ class FuseMountPoint : public MockDaemonHost, public NetFS::FuseApp {
public:
FuseMountPoint() :
MockDaemonHost(::testEndpoint, {"--NetFSD.ConfigPath=" + (rootDir / "defaultDaemon.xml").string()}),
- NetFS::FuseApp({::testUri})
+ NetFS::FuseApp({::testUri}), fargs {}
{
- std::filesystem::remove(mntpnt);
+ std::filesystem::remove_all(mntpnt);
std::filesystem::create_directory(mntpnt);
- struct fuse_args fargs {
- };
+ ::fuse_opt_add_arg(&fargs, selfExe.c_str());
::fuse_opt_add_arg(&fargs, "-onoauto_cache");
::fuse_opt_add_arg(&fargs, "-oattr_timeout=0");
::fuse_opt_add_arg(&fargs, "-oac_attr_timeout=0");
fs = ::fuse_new(&fargs, &operations, sizeof(fuse_operations), this);
BOOST_REQUIRE(fs);
+ }
+
+ void
+ start()
+ {
+ BOOST_REQUIRE(!th);
BOOST_REQUIRE_EQUAL(0, ::fuse_mount(fs, mntpnt.c_str()));
th = std::make_unique<std::thread>(::fuse_loop, fs);
+ BOOST_REQUIRE(th);
}
~FuseMountPoint() override
{
- ::fuse_unmount(fs);
+ stop();
+ std::filesystem::remove(mntpnt);
+ ::fuse_destroy(fs);
+ ::fuse_opt_free_args(&fargs);
+ }
+
+ void
+ stop()
+ {
if (th) {
+ ::fuse_unmount(fs);
th->join();
+ th.reset();
}
- std::filesystem::remove(mntpnt);
}
- FuseMountPoint(const FuseMountPoint &) = delete;
- FuseMountPoint(FuseMountPoint &&) = delete;
-
- FuseMountPoint & operator=(const FuseMountPoint &) = delete;
- FuseMountPoint & operator=(FuseMountPoint &&) = delete;
+ SPECIAL_MEMBERS_DELETE(FuseMountPoint);
struct fuse_context *
fuse_get_context() override
@@ -67,10 +80,14 @@ public:
vlogf(int, const char * fmt, va_list args) const noexcept override
{
std::unique_ptr<char, void (*)(void *)> msg(vstrdupf(fmt, args), std::free);
- BOOST_TEST_MESSAGE(msg.get());
+ static std::mutex btm;
+ ScopeLock(btm) {
+ BOOST_TEST_MESSAGE(msg.get());
+ }
}
struct fuse * fs;
+ struct fuse_args fargs;
std::unique_ptr<std::thread> th;
};
@@ -102,13 +119,31 @@ get_lstat(const std::filesystem::path & p)
BOOST_FIXTURE_TEST_SUITE(fmp, FuseMountPoint);
+class Run {
+public:
+ Run(FuseMountPoint * _p) : p {_p}
+ {
+ p->start();
+ }
+ ~Run()
+ {
+ p->stop();
+ }
+ SPECIAL_MEMBERS_DELETE(Run);
+
+private:
+ FuseMountPoint * p;
+};
+
BOOST_AUTO_TEST_CASE(fuse, *boost::unit_test::timeout(5))
{
+ Run r {this};
BOOST_REQUIRE(std::filesystem::is_directory(mntpnt));
}
BOOST_AUTO_TEST_CASE(fuse_ls, *boost::unit_test::timeout(5))
{
+ Run r {this};
BOOST_REQUIRE(std::filesystem::is_directory(mntpnt));
BOOST_REQUIRE(std::filesystem::is_empty(mntpnt));
const auto rpath = mntpnt / "me";
diff --git a/netfs/unittests/testGlacier.cpp b/netfs/unittests/testGlacier.cpp
index 47782c5..5950e61 100644
--- a/netfs/unittests/testGlacier.cpp
+++ b/netfs/unittests/testGlacier.cpp
@@ -12,7 +12,7 @@ BOOST_AUTO_TEST_CASE(withRouter)
{
std::filesystem::remove(PID);
BOOST_REQUIRE_EQUAL(0,
- system("/usr/bin/glacier2router --Glacier2.Client.Endpoints='tcp -p 14063' "
+ system("flock /tmp/glacier.lock /usr/bin/glacier2router --Glacier2.Client.Endpoints='tcp -p 14063' "
"--Glacier2.PermissionsVerifier=Glacier2/NullPermissionsVerifier --daemon --pidfile " PID));
sleep(1);
diff --git a/netfs/unittests/testLib.cpp b/netfs/unittests/testLib.cpp
index c99f582..7c41340 100644
--- a/netfs/unittests/testLib.cpp
+++ b/netfs/unittests/testLib.cpp
@@ -6,14 +6,20 @@
#include <fuseMappersImpl.h>
#include <lockHelpers.h>
-class TestEntCache : public EntCache<User> {
+struct TestEntry {
+ TestEntry(int i, std::string n) : id(i), name(std::move(n)) { }
+
+ int id;
+ std::string name;
+};
+
+struct TestEntCache : public EntCache<TestEntry> {
void
- fillCache() const noexcept override
+ fillCache() const noexcept
{
- Lock(lock);
- idcache->insert(std::make_shared<User>(1, "user1", 1));
- idcache->insert(std::make_shared<User>(2, "user2", 1));
- idcache->insert(std::make_shared<User>(3, "user3", 1));
+ idcache->insert(std::make_shared<TestEntry>(1, "user1"));
+ idcache->insert(std::make_shared<TestEntry>(2, "user2"));
+ idcache->insert(std::make_shared<TestEntry>(3, "user3"));
}
};
@@ -84,6 +90,7 @@ BOOST_FIXTURE_TEST_CASE(groupcache, TestGroupEntCache)
BOOST_REQUIRE(root);
BOOST_CHECK_EQUAL(root->id, 0);
BOOST_CHECK_EQUAL(root->name, "root");
+ BOOST_CHECK_EQUAL(root->members.size(), 1);
BOOST_CHECK(root->hasMember(0));
auto rootByName = getEntry(root->name);
diff --git a/netfs/unittests/thread-suppressions.txt b/netfs/unittests/thread-suppressions.txt
new file mode 100644
index 0000000..95c4982
--- /dev/null
+++ b/netfs/unittests/thread-suppressions.txt
@@ -0,0 +1,5 @@
+# Unmount Fuse filesystem while daemon is reading socket (expected)
+race:fmp::Run::~Run
+
+# Pfft... no idea
+race:IceInternal::PromiseOutgoing
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-08 20:30:09 +0000