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.

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

sometimes the app has FD leak, when the APP use fileLock #1440

Closed
qclzdh opened this issue Jun 30, 2022 · 7 comments · Fixed by #1442
Closed

sometimes the app has FD leak, when the APP use fileLock #1440

qclzdh opened this issue Jun 30, 2022 · 7 comments · Fixed by #1442
Assignees
Labels
bug Something isn't working

Comments

@qclzdh
Copy link

qclzdh commented Jun 30, 2022

Required information

Operating system:
E.g. Ubuntu 18.04 LTS

Compiler version:
E.g. GCC 7.4.0

Observed result or behaviour:
There are some FD leakage, it could easily reach to the max opened FDs of a process in Linux platform.

Expected result or behaviour:
no FD leakage.

Conditions where it occurred / Performed steps:

APP_A

int main()
{
    //sending();

    iox::runtime::PoshRuntime::initRuntime(APP_NAME);
    auto maybeFileLock = iox::posix::FileLock::create(TEST_NAME);


    while(!iox::posix::hasTerminationRequested()) {
        std::this_thread::sleep_for(std::chrono::seconds(1));
    }

    return (EXIT_SUCCESS);
}

APP_B

int main()
{
    iox::runtime::PoshRuntime::initRuntime(APP_NAME);

    constexpr char TEST_NAME[] = "TestProcessName0630";

    for (int i = 0; i < 1024; i++)
    {
        auto maybeFileLock = iox::posix::FileLock::create(TEST_NAME);

        if (!maybeFileLock.has_error()) {
            // process has exit
            std::cout << "publisher has exit" << std::endl;
        } else {
            if (maybeFileLock.get_error() == iox::posix::FileLockError::LOCKED_BY_OTHER_PROCESS) {
                std::cout << "file is locked by another process" << std::endl;
            } else {
                // normally, this shouldn't happend
                std::cout << "Error occured while acquiring file lock named " << std::endl;
            }
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(50));
    }

{
        auto maybeFileLock = iox::posix::FileLock::create(TEST_NAME);

        if (!maybeFileLock.has_error()) {
            // process has exit
            std::cout << "publisher has exit" << std::endl;
        } else {
            if (maybeFileLock.get_error() == iox::posix::FileLockError::LOCKED_BY_OTHER_PROCESS) {
                std::cout << "file is locked by another process" << std::endl;
            } else {
                // normally, this shouldn't happend
                std::cout << "Error occured while acquiring file lock named " << std::endl;
            }
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(50));
}


    return (EXIT_SUCCESS);
}

and start APP_A first, then start APP_B.
You will see this issue.

@qclzdh
Copy link
Author

qclzdh commented Jun 30, 2022

it seems this issue only can be seen in 2.0.x, master branch has fixed this.

@elfenpiff
Copy link
Contributor

@qclzdh how is the TEST_NAME defined for APP_A, does it have the same value as APP_B?

@qclzdh
Copy link
Author

qclzdh commented Jun 30, 2022

@elfenpiff Sorry for incomplete code, TEST_NAME could be any string, as long as they are same in APP_A and APP_B. I do the test with following definition

constexpr char TEST_NAME[] = "TestProcess0630";

@qclzdh
Copy link
Author

qclzdh commented Jun 30, 2022

@qclzdh how is the TEST_NAME defined for APP_A, does it have the same value as APP_B?

@elfenpiff
Sorry, There is a TEST_NAME defined to "TestProcessName0630", so please make sure they are defined to same one APP_A and APP_B.
Or, maybe you can modify in the test_posix_file_lock.cpp, it could easily reproduce the issue.

// TEST_F(FileLock_test, CreatingSameFileLockAgainFails)
// {
//     ::testing::Test::RecordProperty("TEST_ID", "ed3af1c8-4a84-4d4f-a267-c4a80481dc42");
//     auto sut2 = iox::posix::FileLock::create(TEST_NAME);
//     ASSERT_TRUE(sut2.has_error());
//     EXPECT_THAT(sut2.get_error(), Eq(FileLockError::LOCKED_BY_OTHER_PROCESS));
// }

TEST_F(FileLock_test, DoesNotReachToTheLinuxProcessMaxFile)
{
    ::testing::Test::RecordProperty("TEST_ID", "ed3af1c8-4a84-4d4f-a267-c4a80481dc42");
    for (int i = 0; i < 65536; i++) {
        auto sut2 = iox::posix::FileLock::create(TEST_NAME);
        ASSERT_TRUE(sut2.has_error());
        EXPECT_THAT(sut2.get_error(), Eq(FileLockError::LOCKED_BY_OTHER_PROCESS));
    }
}

@elfenpiff
Copy link
Contributor

@qclzdh Thanks for the test it was very helpful to reproduce the bug.

Now the question is, is it important for you that we release a bug fix 2.0 release or can you work with master? If you could work with master I would create only a PR which fixes the issue and whenever we release a 2.0.3 this fix would be added.

@elfenpiff elfenpiff self-assigned this Jun 30, 2022
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jun 30, 2022
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jun 30, 2022
Signed-off-by: Christian Eltzschig <[email protected]>
@elBoberido elBoberido linked a pull request Jun 30, 2022 that will close this issue
21 tasks
@qclzdh
Copy link
Author

qclzdh commented Jul 1, 2022

@elfenpiff Thanks, master is fine~

@mossmaurice mossmaurice added the bug Something isn't working label Jul 6, 2022
elfenpiff added a commit to ApexAI/iceoryx that referenced this issue Jul 12, 2022
elBoberido added a commit that referenced this issue Jul 14, 2022
…release-2.0

Iox #1440 fix file lock leak in release 2.0
@elfenpiff
Copy link
Contributor

Fixed on master with the new builder pattern and on release 2.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants