0

When having multiple threads using shared data, how does one correctly handle the destruction of that data when exceptions are thrown?

I'm working on an application where I want one process to be doing work while waiting for a result sent from another process over a network. I implemented a class that creates a thread to wait for the result, something like the following (actual code is too large to post here, so this is a small example):

class Notifier {
  public: 
    Notifier(Client* _client) : value(-1), client(_client) {
      listener = boost::thread(&Notifer::Listen, this);
    }

    void Listen() {
      try {
        int rec = client->Receive(); //blocking call to receive data over a socket
        boost::scoped_lock sl(mutex);
        value = rec;
      } catch (...) {
        cout << "Exception thrown in listener" << endl;
      }
    }

    int Get() {
      boost::mutex::scoped_lock sl(mutex);
      return value;
    }

  private:
    int value;
    boost::mutex; 
    boost::thread listener;
    Client* client;
}

int main() {
  Client client; //initialize client, connect, etc.
  Notifier n(client);
  while(n.Get() < 0) {
   // do work
   cout << "Waiting for data" << endl;
   sleep(1);
  }
}

This works fine for me until I add exception handling:

int main() {
  try {
    Client client; //initialize client, connect, etc.
    Notifier n(client);
    while(n.Get() < 0) {
     // do work
     cout << "Waiting for data" << endl;
     sleep(1);
     throw exception();
    }
  } catch(...) {
    cout << "Exception thrown in main" << endl;
  }
  return 0;
}

I get the error

"boost: mutex lock failed in pthread_mutex_lock: Invalid argument".

My guess is that, when the exception gets thrown, the stack for the main function gets unwound, destroying the mutex. Then the Receive() call in the Listen function returns and the scoped_lock constructor tries to lock the mutex, which doesn't exist, resulting in the error.

Can someone confirm that this is indeed what is happening? If so, is there a way to communicate to the thread that that the mutex no longer exists or that the thread should terminate? Or is there a more exception-safe way of doing what I'm trying to do?

maditya
  • 8,626
  • 2
  • 28
  • 28

1 Answers1

0

You have to write a correct destructor for Notifier class, which would cancel all blocked calls (i.e. unblock client->Receive()) and then terminate listener thread. Also you need to modify Notifier::Listen() to periodically check for thread termination request... then everything will be Ok.

zaufi
  • 6,811
  • 26
  • 34
  • That's an interesting idea as long as I can actually cancel the blocked call. My Client class uses the Unix `recv` function, do you know how this can be unblocked? – maditya Jul 26 '13 at 04:27
  • Found it - there is a [`shutdown`](http://linux.die.net/man/2/shutdown) function. Thanks! I had no idea this was possible. – maditya Jul 26 '13 at 05:13
  • I have one more question. If I use the destructor of the Notifier as you suggest, I would be doing e.g. `client->Shutdown()`. But this client is not really "owned" by the Notifier, it's passed in as an argument and in a more complex program it could be used elsewhere after the Notifier has gone out of scope. In this scenario, I don't want to shut down the client. Is there a way to differentiate between a normal destruction and one caused by exceptions so I only shutdown the client in the second scenario? – maditya Jul 26 '13 at 05:14
  • I don't understand precisely design of your application (including a "complex" one), but I wonder: 0) why don't you use boost::asio; 1) another "problem" for me w/ this design: you have `Notifier` which is coupled tight w/ `Client`; 2) why not to use completely asynchronous I/O w/ a pool of workers? – zaufi Jul 26 '13 at 05:40
  • The Client is actually legacy code from a previous programmer, which I am still phasing out. I have a deadline in a few days and won't be able to do it before then, so I'm just trying to work with what I have now. I agree that the Client and Notifier shouldn't be tightly coupled. Could you elaborate more on the pool of workers? – maditya Jul 26 '13 at 05:48
  • Consider this approach: 0) `Client` (using `boost::asio`) do completely asynchronous I/O 1) ... yep, you need to start a bunch of threads (running a boost::io_service::run() in a forever loop, handling for thread termination exception) 2) on data arrival `Client` emit a signal (using boost::signals2) w/ signature like `void (const std::string&)` 3) anyone can subscribe to that signal (or any others, like 'IO-failure'), so you just __don't need any special class to notify__ – zaufi Jul 26 '13 at 05:49
  • huh, legacy :( anyway, you'd better to switch to non-blocking I/O, and asynchronous notifications (from a `Client` class using `boost::signals2`) -- as for me that is the easiest way – zaufi Jul 26 '13 at 05:52
  • Interesting, this basically combines the signal/slot mechanism to communication over a network. I'm not experienced with boost's signal/slot library but I'll definitely give this a try. Also thank you for the original answer – maditya Jul 26 '13 at 06:05