sewenew / redis-plus-plus

Redis client written in C++
Apache License 2.0
1.64k stars 351 forks source link

[QUESTION] The program will be terminated when exception occurs during the automatic unlocking #563

Closed regenttsui closed 4 months ago

regenttsui commented 6 months ago

Describe the problem If an exception occurs during the automatic unlocking in the destructor of std::lock_guard,it seems to cause the program to terminate. Here is an example, I manually shutdown the Redis before the deconstruction of std::lock_guard:

#include <memory>
#include <sw/redis++/redis++.h>
#include <sw/redis++/patterns/redlock.h>
#include <iostream>

using namespace sw::redis;

int main() {
        try {
                ConnectionOptions connection_options;
                connection_options.host = "127.0.0.1";
                connection_options.port = 9379;
                auto redis = std::make_shared<Redis>(connection_options);
                RedMutex mtx(redis, "resource");
                std::lock_guard<RedMutex> lock(mtx);
        //Sleep for a while to shutdown Redis manually
                std::cout << "sleep for 10s" << std::endl;
                std::this_thread::sleep_for(std::chrono::seconds(10));
        } catch (const Error &err) {
                std::cout << "catch error=" << err.what() << std::endl;
        }
        std::cout << "success" << std::endl;

        return 0;
}

I think the reason is that the exception is re-thrown in the destructor. Here is the source code of redispp:

void RedMutexImpl::unlock() {
    std::lock_guard<std::mutex> lock(_mtx);

    if (!_locked()) {
        throw Error("RedMutex is not locked");
    }

    try {
        _unlock(_lock_id);
    } catch (...) {
        _reset();
        throw;
    }

    _reset();
}

Environment:

sewenew commented 6 months ago

You used a very old version of redis-plus-plus. Please update to the latest one, and try again. This problem should have been fixed.

regenttsui commented 6 months ago

You used a very old version of redis-plus-plus. Please update to the latest one, and try again. This problem should have been fixed.

I tried the version 1.3.12, but the problem still exists

sewenew commented 6 months ago

I manually shutdown the Redis before the deconstruction of std::lock_guard

Sorry, I missed this. Yes, this is a problem. Looks like that unlock should not throw. However, in this case, application which calls unlock directly, cannot tell if unlock successes or not.

Thanks for reporting it! I'll rethink it, and fix it.

Regards

sewenew commented 4 months ago

I checked std::mutex::unlock implementation, in fact, it also might throw in some edge cases, cases that seldom happen. However, RedMutex need to do network operations to do unlock, and it might fail once the connection is broken. So I removed the code that throws exception to ensure the program does not terminate in that case.

Problem has been fixed, please try the latest code on master branch.

Thanks again for reporting this bug! And sorry for the late fix.

Regards