0

I implemented a lock macro based on boost mutex, the code is below:

#include <boost/thread.hpp>
#include <iostream>

#define LOCK(x) if(Lock _lock_=x){}else

class Mutex{
public:
    friend class Lock;

private:
    boost::mutex mutex_;

    void Lock(){
        mutex_.lock();
    };

    void Unlock(){
        mutex_.unlock();
    };
};

class Lock{
public:
    Lock(Mutex& mutex):mutex_(mutex){mutex_.Lock();};
    ~Lock(){mutex_.Unlock();};

    operator bool() const {
        return false;
    }

private:
    Mutex& mutex_;
};

void wait(int seconds)
{
    boost::this_thread::sleep(boost::posix_time::seconds(seconds));
}

Mutex mtx;

void thread()
{
    for (int i = 0; i < 5; ++i)
    {
        LOCK(mtx){
            wait(1);
            std::cout << "Thread " << boost::this_thread::get_id() << ": " << i << std::endl;
        }
    }
}

int main()
{
    boost::thread t1(thread);
    boost::thread t2(thread);
    t1.join();
    t2.join();
}

I compiled it in the Mac OS using clang++ -std=c++11 -stdlib=libc++ lock_raii.cc -lboost_system -lboost_thread. When I run it, there is a Segmentation fault: 11.

What's the problem with it?

injoy
  • 3,993
  • 10
  • 40
  • 69
  • Did you try debugging it? – Carl Norum Oct 09 '13 at 21:29
  • 4
    Please don't use that macro. It's ugly. – GManNickG Oct 09 '13 at 21:30
  • 2
    C++11 provides standard mutex and locking constructs; see [lock_guard](http://en.cppreference.com/w/cpp/thread/lock_guard). Your macro trick is clever, but not good practice. It adds another layer of indirection and cognitive friction for virtually no gain. – Marcelo Cantos Oct 09 '13 at 21:33
  • Sorry, i ment this: http://stackoverflow.com/questions/18158381/python-crashing-when-running-two-commands – Patrick Bassut Oct 09 '13 at 21:35
  • 1
    Is there any reason for you not to use lock_guard from either C++11 or boost? I find that macro "trick" particularly nasty... – namezero Oct 09 '13 at 22:22
  • 3
    Why are you reinventing a mutex when you have `boost::mutex`? Why are you reinventing a lock type when you have `boost::scoped_lock`? If you really want to make that macro work (which is a bad idea IMHO) then you need to make the `Lock` type movable (not copyable) and make it not unlock anything in the destructor if ownership of the lock has moved to another object, then you're guaranteed only one object owns the lock and only one will unlock it. – Jonathan Wakely Oct 09 '13 at 22:49

1 Answers1

3

Aside of the fact that this method at least questionable, I do not see a problem with the code. gcc v4.7.0 on Linux does not have issues either (there is no segmentation fault). So you may not setup boost properly or have a bug somewhere. You should run your program under debugger and see where the problem occurs. In theory this code:

if(Lock _lock_=x){}else

could be compiled as:

if(Lock _lock_= Lock(x) ){}else

and invoking copy ctor and have double unlock on the mutex. To make sure this is not the issue make copy ctor for class Lock private.

Slava
  • 43,454
  • 1
  • 47
  • 90
  • If you make the copy constructor private then `Lock _lock_=x` is ill-formed and will not compile. In short, the design is broken, the solution is to change it. – Jonathan Wakely Oct 09 '13 at 22:43