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

exception safe locks/unlocks #26

Closed
ovanes opened this issue Jun 25, 2015 · 2 comments
Closed

exception safe locks/unlocks #26

ovanes opened this issue Jun 25, 2015 · 2 comments

Comments

@ovanes
Copy link

ovanes commented Jun 25, 2015

I just wonder, why sometimes is explicit locking on a mutex object is used instead of a unique_lock with explicit unlock call.

In the file: redis3m/src/connection_pool.cpp or redis3m/src/simple_pool.cpp I code like that:

connection::ptr_t simple_pool::get()
{
    connection::ptr_t ret;

    access_mutex.lock();
    std::set<connection::ptr_t>::iterator it = connections.begin();
    if (it != connections.end())
    {
        ret = *it;
        connections.erase(it);
    }

    access_mutex.unlock();

    if (!ret)
    {
        ret = connection::create(_host, _port);
        // Setup connections selecting db
        if (_database != 0)
        {
            ret->run(command("SELECT") << _database);
        }
    }
    return ret;
}

Why not writing it like:

connection::ptr_t simple_pool::get()
{
    connection::ptr_t ret;

    std::unique_lock<std::mutex> lock(access_mutex);
    std::set<connection::ptr_t>::iterator it = connections.begin();
    if (it != connections.end())
    {
        ret = *it;
        connections.erase(it);
    }

    lock.unlock();

    if (!ret)
    {
        ret = connection::create(_host, _port);
        // Setup connections selecting db
        if (_database != 0)
        {
            ret->run(command("SELECT") << _database);
        }
    }
    return ret;
}

In fact unique_lock is used 30 lines below...

I understand that set::erase(iterator) version has the nothrow guarantee, but still I think consistency would be better. And there is no performance drawback in that case.

@ovanes ovanes changed the title thread safe locks exception safe locks/unlocks Jun 25, 2015
@luca3m
Copy link
Owner

luca3m commented Jun 29, 2015

Usually I use explicit lock instead of unique_lock when I need to unlock it before the scope exits. It can be achieved also using unique_lock so I agree with you, to be more consistent.

@luca3m
Copy link
Owner

luca3m commented Aug 11, 2015

#27 pull request, that I've just merge should address this

@luca3m luca3m closed this as completed Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants