std::promise set_exception twice cause Segmentation fault

Question

Let's say I have a method that calls an unstable third-party service, so I add a timeout for this call say 10 seconds. Here is what I tried:

int process()
{
    std::promise<int> promise;
    std::future<int> future = promise.get_future();

    std::thread([&]
    {
        try
        {
            int result = call_third_party_service();
            promise.set_value(result);
        }
        catch (std::exception&) //call_thrid_party_service can throw exceptions
        {
            promise.set_exception(std::current_exception());
        }
    }).detach();

    auto status = future.wait_for(std::chrono::seconds(10));
    if (status == std::future_status::timeout)
    {
        promise.set_exception(time_out_exception);
    }

    return future.get();
}

int main()
{
    try
    {
        int result = process();
    }
    catch(const std::exception& e)
    {
        //print
    }

    //blocks the thread to see what happens
    std::this_thread::sleep_for(std::chrono::minutes(1));        
    return 0;
}

When call_third_party_service is not responding (assume it throws an exception saying timeout after 30 seconds), status == std::future_status::timeout hits after 10 seconds' waiting, then promise.set_exception works, and everything looks good. However when call_third_party_service throws the exception, promise.set_exception again, hence the Segmentation fault. What's the correct way to implement this pattern?

The problem here is that after your timeout the `process` function returns, which means that `promise` ends its life-time and will be destructed. That leaves the thread with a reference to an object that no longer exist. You need to come up with a way to keep the promise alive during the whole life-time of the thread. — Some programmer dude, Apr 30 '19 at 07:11
Don't call set_exception in the case of the timeout, just throw the exception directly. — Frax, Apr 30 '19 at 07:13
@Frax Still got segmentation fault. I think the reason is like @Some programmer dude says, `promise` is out of its life cycle. — Cheng Chen, Apr 30 '19 at 07:15
@Someprogrammerdude What do you suggest then? Putting it inside a global container and remove periodically? — Cheng Chen, Apr 30 '19 at 07:16
Perhaps using `std::shared_ptr` and capture it by value? I haven't really had time to think about it in that much detail. — Some programmer dude, Apr 30 '19 at 07:23
Yeah, throwing the exception directly removes the duplicated call (and makes the code easier to read, really). I read the documentation a bit. You should move your promise to the other thread (the writing end; the promise implements move semantics), and keep only the future on the reading end, no need for shared_ptr. — Frax, Apr 30 '19 at 08:17

Mike van Dyke · Answer 1 · 2019-04-30T11:04:40.963

As suggested by Frax, you should move the promise into the lambda and throw the exception directly when the future times out:

int process() {
  std::promise<int> promise;
  std::future<int> future = promise.get_future();

  // move ownership of the promise into thread
  std::thread([prom = std::move(promise)]() mutable {
    try {
      int result = call_third_party_service();
      prom.set_value(result);
    } catch (std::exception&)  // call_thrid_party_service can throw exceptions
    {
      prom.set_exception(std::current_exception());
    }
  }).detach();

  auto status = future.wait_for(std::chrono::seconds(10));
  if (status == std::future_status::timeout) {
    // This exception is not part of an asynchronous computation and 
    // should be thrown immediately
    throw time_out_exception("timed out");
  }

  return future.get();
}

int main() {
  try {
    int result = process();
  } catch (const std::exception& e) {
    // print
  }

  // blocks the thread to see what happens
  std::this_thread::sleep_for(std::chrono::minutes(1)); 
  return 0;
}

std::promise set_exception twice cause Segmentation fault

1 Answers1