Title
promise::set_value() and promise::get_future() should not race
Status
c++20
Section
[futures.promise] [futures.task.members]
Submitter
Jonathan Wakely

Created on 2014-06-23.00:00:00 last changed 1 month ago

Messages

Date: 2018-11-12.04:39:29

Proposed resolution:

This wording is relative to N4750.

  1. Change [futures.promise] around p12 as indicated:

    future<R> get_future();
    

    -12- Returns: A future<R> object with the same shared state as *this.

    -?- Synchronization: Calls to this function do not introduce data races ([intro.multithread]) with calls to set_value, set_exception, set_value_at_thread_exit, or set_exception_at_thread_exit. [Note: Such calls need not synchronize with each other. — end note]

    -13- Throws: future_error if *this has no shared state or if get_future has already been called on a promise with the same shared state as *this.

    -14- Error conditions: […]

  2. Change [futures.task.members] around p13 as indicated:

    future<R> get_future();
    

    -13- Returns: A future object that shares the same shared state as *this.

    -?- Synchronization: Calls to this function do not introduce data races ([intro.multithread]) with calls to operator() or make_ready_at_thread_exit. [Note: Such calls need not synchronize with each other. — end note]

    -14- Throws: a future_error object if an error occurs.

    -15- Error conditions: […]

Date: 2018-11-12.04:39:29

[ 2018-11, Adopted in San Diego ]

Date: 2018-06-07.20:35:06

[ 2018-06, Rapperswil, Wednesday evening session ]

JW: lets move on and I'll file another issue to make the wording better
BO: the current wording is better than what there before
JM: ACTION I should file an editorial issue to clean up on how to refer to [res.on.data.races]: raised editorial issue 2097
ACTION: move to Ready

Daniel rebases wording to N4750.

Date: 2017-03-15.00:00:00

[ 2017-03-01, Kona, SG1 ]

GeoffR to forward revised wording.

Date: 2017-02-15.00:00:00

[ 2017-02-28, Kona ]

SG1 has updated wording for LWG 2412. SG1 voted to move this to Ready status by unanimous consent.

Date: 2016-10-15.00:00:00

[ 2016-10-21, Nico comments ]

After creating a promise or packaged task one thread can call get_future() while another thread can set values/exceptions (either directly or via function call). This happens very easily.

Consider:

promise<string> p;
thread t(doSomething, ref(p));
cout << "result: " << p.get_future().get() << endl;

AFAIK, this is currently UB due to a data race (calling get_future() for the promise might happen while setting the value in the promise).

Yes, a fix is pretty easy:

promise<string> p;
future<string> f(p.get_future());
thread t(doSomething, ref(p));
cout << "result: " << f.get() << endl;

but I would like to have get_future() and setters be synchronized to avoid this UB.

This would especially make the use of packaged tasks a lot easier. Consider:

vector<packaged_task<int(char)>> tasks;
packaged_task<int(char)> t1(func);

// start separate thread to run all tasks:
auto futCallTasks = async(launch::async, callTasks, ref(tasks));

for (auto& fut : tasksResults) {
  cout << "result: " << fut.get_future().get() << endl; // OOPS: UB
}

Again, AFAIK, this program currently is UB due to a data race. Instead, currently I'd have to program, which is a lot less convenient:

vector<packaged_task<int(char)>> tasks;
vector<future<int>> tasksResults;
packaged_task<int(char)> t1(func);
tasksResults.push_back(t1.getFuture()));
tasks.push_back(move(t1));

// start separate thread to run all tasks:
auto futCallTasks = async(launch::async, callTasks, ref(tasks));

for (auto& fut : tasksResults) {
  cout << "result: " << fut.get() << endl;
}

With my naive thinking I see not reason not to guarantee that these calls synchronize (as get_future returns an "address/reference" while all setters set the values there).

Previous resolution [SUPERSEDED]:

This wording is relative to N3936.

  1. Change [futures.promise] around p12 as indicated:

    future<R> get_future();
    

    -12- Returns: A future<R> object with the same shared state as *this.

    -?- Synchronization: Calls to this function do not conflict ([intro.multithread]) with calls to set_value, set_exception, set_value_at_thread_exit, or set_exception_at_thread_exit. [Note: Such calls need not be synchronized, but implementations must ensure they do not introduce data races. — end note]

    -13- Throws: future_error if *this has no shared state or if get_future has already been called on a promise with the same shared state as *this.

    -14- Error conditions: […]

  2. Change [futures.task.members] around p13 as indicated:

    future<R> get_future();
    

    -13- Returns: A future<R> object that shares the same shared state as *this.

    -?- Synchronization: Calls to this function do not conflict ([intro.multithread]) with calls to operator() or make_ready_at_thread_exit. [Note: Such calls need not be synchronized, but implementations must ensure they do not introduce data races. — end note]

    -14- Throws: a future_error object if an error occurs.

    -15- Error conditions: […]

Date: 2015-04-04.16:45:17

[ 2015-02 Cologne ]

Handed over to SG1.

Date: 2014-06-23.00:00:00

The following code has a data race according to the standard:

std::promise<void> p;
std::thread t{ []{
  p.get_future().wait();
}};
p.set_value();
t.join();

The problem is that both promise::set_value() and promise::get_future() are non-const member functions which modify the same object, and we only have wording saying that the set_value() and wait() calls (i.e. calls setting and reading the shared state) are synchronized.

The calls don't actually access the same memory locations, so the standard should allow it. My suggestion is to state that calling get_future() does not conflict with calling the various functions that make the shared state ready, but clarify with a note that this does not imply any synchronization or "happens before", only being free from data races.

History
Date User Action Args
2021-02-25 10:48:01adminsetstatus: wp -> c++20
2018-11-12 04:39:29adminsetmessages: + msg10178
2018-11-12 04:39:29adminsetstatus: voting -> wp
2018-10-08 05:13:59adminsetstatus: ready -> voting
2018-06-07 20:35:06adminsetmessages: + msg9873
2018-06-07 20:35:06adminsetstatus: review -> ready
2017-03-01 18:09:34adminsetmessages: + msg9030
2017-02-28 21:45:14adminsetstatus: open -> review
2017-02-28 21:12:50adminsetstatus: ready -> open
2017-02-28 20:58:29adminsetstatus: open -> ready
2017-02-28 19:03:27adminsetmessages: + msg9015
2016-11-01 19:25:11adminsetmessages: + msg8577
2015-04-04 16:45:17adminsetmessages: + msg7337
2015-04-04 16:45:17adminsetstatus: new -> open
2014-06-29 12:11:20adminsetmessages: + msg7085
2014-06-23 00:00:00admincreate