Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix dead lock in ExportPerformer #503

Merged
merged 3 commits into from
Jun 11, 2024
Merged

fix dead lock in ExportPerformer #503

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d9b498
fix dead lock in ExportPerformer
HanLee13 Jun 6, 2024
3610ec5
fix unittest ExportFS.op_failed_situation
HanLee13 Jun 7, 2024
462f0a1
remove ctor of AsyncWaiter
HanLee13 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions fs/async_filesystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,12 @@ namespace fs
struct AsyncWaiter
{
std::mutex _mtx;
std::unique_lock<std::mutex> _lock;
std::condition_variable _cond;
bool _got_it = false;
typename AsyncResult<R>::result_type ret;
int err = 0;

AsyncWaiter() : _lock(_mtx) { }
AsyncWaiter() { }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for a ctor anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

int on_done(AsyncResult<R>* r)
{
std::lock_guard<std::mutex> lock(_mtx);
Expand All @@ -156,8 +155,9 @@ namespace fs
}
R wait()
{
std::unique_lock<std::mutex> lock(_mtx);
while(!_got_it)
_cond.wait(_lock, [this]{return _got_it;});
_cond.wait(lock, [this]{return _got_it;});
if (err) errno = err;
return (R)ret;
}
Expand Down
4 changes: 4 additions & 0 deletions fs/exportfs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ namespace fs
{
PERFORM(OPID_APPENDV, m_file->do_appendv(iov, iovcnt, offset, position));
}
OVERRIDE_ASYNC(int, fallocate, int mode, off_t offset, off_t len)
{
PERFORM(OPID_FALLOCATE, m_file->fallocate(mode, offset, len));
}
OVERRIDE_ASYNC(ssize_t, fgetxattr, const char *name, void *value, size_t size)
{
if (!m_xattr) {
Expand Down
6 changes: 3 additions & 3 deletions fs/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ add_executable(test-fs test.cpp)
target_link_libraries(test-fs PRIVATE photon_shared)
add_test(NAME test-fs COMMAND $<TARGET_FILE:test-fs>)

# add_executable(test-exportfs test_exportfs.cpp)
# target_link_libraries(test-exportfs PRIVATE photon_shared)
# add_test(NAME test-exportfs COMMAND $<TARGET_FILE:test-exportfs>)
add_executable(test-exportfs test_exportfs.cpp)
target_link_libraries(test-exportfs PRIVATE photon_shared)
add_test(NAME test-exportfs COMMAND $<TARGET_FILE:test-exportfs>)

add_executable(test-filecopy test_filecopy.cpp)
target_link_libraries(test-filecopy PRIVATE photon_shared)
Expand Down
20 changes: 14 additions & 6 deletions fs/test/mock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace PMock {
using photon::fs::DIR;
using photon::fs::fiemap;

class MockNullFile : public IFile, public IFileXAttr {
class MockNoAttrNullFile : public IFile {
public:
MOCK_METHOD0(filesystem, IFileSystem*());
MOCK_METHOD3(pread, ssize_t(void*, size_t, off_t));
Expand All @@ -49,13 +49,9 @@ namespace PMock {
MOCK_METHOD2(trim, int(off_t, off_t));
MOCK_METHOD1(fiemap, int(struct fiemap *p));
MOCK_METHOD2(vioctl, int(int, va_list));
MOCK_METHOD3(fgetxattr, ssize_t(const char*, void*, size_t));
MOCK_METHOD2(flistxattr, ssize_t(char*, size_t));
MOCK_METHOD4(fsetxattr, int(const char*, const void*, size_t, int));
MOCK_METHOD1(fremovexattr, int(const char*));
};

class MockNullFileSystem : public IFileSystem, public IFileSystemXAttr{
class MockNoAttrNullFileSystem : public IFileSystem {
public:
MOCK_METHOD2(open, IFile*(const char *pathname, int flags));
MOCK_METHOD3(open, IFile*(const char *pathname, int flags, mode_t mode));
Expand All @@ -82,6 +78,18 @@ namespace PMock {
MOCK_METHOD3(mknod, int(const char *path, mode_t mode, dev_t dev));
MOCK_METHOD0(syncfs, int());
MOCK_METHOD1(opendir, DIR*(const char *name));
};

class MockNullFile : public MockNoAttrNullFile, public IFileXAttr {
public:
MOCK_METHOD3(fgetxattr, ssize_t(const char*, void*, size_t));
MOCK_METHOD2(flistxattr, ssize_t(char*, size_t));
MOCK_METHOD4(fsetxattr, int(const char*, const void*, size_t, int));
MOCK_METHOD1(fremovexattr, int(const char*));
};

class MockNullFileSystem : public MockNoAttrNullFileSystem, public IFileSystemXAttr{
public:
MOCK_METHOD4(getxattr, ssize_t(const char*, const char*, void*, size_t));
MOCK_METHOD4(lgetxattr, ssize_t(const char*, const char*, void*, size_t));
MOCK_METHOD3(listxattr, ssize_t(const char*, char*, size_t));
Expand Down
66 changes: 58 additions & 8 deletions fs/test/test_exportfs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ limitations under the License.
#include <thread>
#include <utime.h>
#include <sys/time.h>
// #include <sys/sysmacros.h>
#if defined(__linux__)
#include <sys/sysmacros.h>
#else
#include <sys/types.h>
#endif
#include "../../test/gtest.h"

using namespace photon;
Expand Down Expand Up @@ -104,6 +108,8 @@ int callbackvoid(void*, AsyncResult<void>* ret) {
}

TEST(ExportFS, basic) {
photon_init();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not doing init in main()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other tests like ExportFS.no_xattr_sync run in pthread rather than photon thread.

DEFER(photon_fini());
PMock::MockNullFile* mockfile = new PMock::MockNullFile();
PMock::MockNullFileSystem* mockfs = new PMock::MockNullFileSystem();
PMock::MockNullDIR* mockdir = new PMock::MockNullDIR();
Expand Down Expand Up @@ -249,6 +255,8 @@ TEST(ExportFS, basic) {
}

TEST(ExportFS, init_fini_failed_situation) {
photon_init();
DEFER(photon_fini());
auto _o_output = log_output;
log_output = log_output_null;
DEFER({
Expand All @@ -270,6 +278,8 @@ TEST(ExportFS, init_fini_failed_situation) {
}

TEST(ExportFS, op_failed_situation) {
photon_init();
DEFER(photon_fini());
auto _o_output = log_output;
log_output = log_output_null;
DEFER({
Expand All @@ -284,18 +294,21 @@ TEST(ExportFS, op_failed_situation) {
delete file;
});

auto action = [=](AsyncResult<ssize_t>* ret){
std::atomic<int> error {0};
auto action = [&](AsyncResult<ssize_t>* ret){
EXPECT_EQ(ENOSYS, ret->error_number);
errno = EDOM;
error = EDOM;
return -1;
};
Callback<AsyncResult<ssize_t>*> fail_cb(action);
file->read(nullptr, 0, fail_cb);
while (EDOM != errno) photon::thread_yield();
EXPECT_EQ(EDOM, errno);
while (EDOM != error) photon::thread_yield();
EXPECT_EQ(EDOM, error);
}

TEST(ExportFS, xattr) {
photon_init();
DEFER(photon_fini());
PMock::MockNullFile* mockfile = new PMock::MockNullFile();
PMock::MockNullFileSystem* mockfs = new PMock::MockNullFileSystem();
auto file = dynamic_cast<IAsyncFileXAttr*>(export_as_async_file(mockfile));
Expand Down Expand Up @@ -347,7 +360,7 @@ TEST(ExportFS, xattr) {
#undef CALL_TEST
#undef CALL_TEST0

TEST(ExportFS, xattr_sync) {
TEST(ExportFS, DISABLED_xattr_sync) {
photon::semaphore sem;
PMock::MockNullFile* mockfile = new PMock::MockNullFile();
PMock::MockNullFileSystem* mockfs = new PMock::MockNullFileSystem();
Expand Down Expand Up @@ -401,11 +414,48 @@ TEST(ExportFS, xattr_sync) {
th.join();
}

TEST(ExportFS, no_xattr_sync) {
photon::semaphore sem;
PMock::MockNoAttrNullFile* mockfile = new PMock::MockNoAttrNullFile();
PMock::MockNoAttrNullFileSystem* mockfs = new PMock::MockNoAttrNullFileSystem();

IFileXAttr* file = nullptr;
IFileSystemXAttr* fs = nullptr;

std::thread th([&]{
photon_init();
DEFER(photon_fini());
file = dynamic_cast<IFileXAttr*>(export_as_sync_file(mockfile));
fs = dynamic_cast<IFileSystemXAttr*>(export_as_sync_fs(mockfs));
sem.wait(1);
DEFER({
delete file;
delete fs;
});
});

while (!file || !fs) { ::sched_yield(); }

EXPECT_EQ(-1, file->fgetxattr(nullptr, nullptr, 0));
EXPECT_EQ(-1, file->flistxattr(nullptr, 0));
EXPECT_EQ(-1, file->fsetxattr(nullptr, nullptr, 0, 0));
EXPECT_EQ(-1, file->fremovexattr(nullptr));

EXPECT_EQ(-1, fs->getxattr(nullptr, nullptr, nullptr, 0));
EXPECT_EQ(-1, fs->listxattr(nullptr, nullptr, 0));
EXPECT_EQ(-1, fs->setxattr(nullptr, nullptr, nullptr, 0, 0));
EXPECT_EQ(-1, fs->removexattr(nullptr, nullptr));
EXPECT_EQ(-1, fs->lgetxattr(nullptr, nullptr, nullptr, 0));
EXPECT_EQ(-1, fs->llistxattr(nullptr, nullptr, 0));
EXPECT_EQ(-1, fs->lsetxattr(nullptr, nullptr, nullptr, 0, 0));
EXPECT_EQ(-1, fs->lremovexattr(nullptr, nullptr));

sem.signal(1);
th.join();
}

int main(int argc, char **argv)
{
photon_init();
DEFER(photon_fini());
::testing::InitGoogleTest(&argc, argv);
int ret = RUN_ALL_TESTS();
LOG_ERROR_RETURN(0, ret, VALUE(ret));
Expand Down
Loading