search

cplusplus / sender-receiver

Issues list for P2300
Apache License 2.0
20 stars 3 forks source link

std::in_place_stop_source::get_token and std::get_stop_token const correctness #31

Open ericniebler opened 10 months ago

ericniebler commented 10 months ago

Issue by jtsylve Thursday Aug 17, 2023 at 20:39 GMT _Originally opened as https://github.com/brycelelbach/wg21_p2300_execution/issues/46_

In the current proposal, in_place_stop_source::get_token is const qualified, and std::get_stop_token requires a const parameter.

The problem in practice is that token callbacks almost certainly need to register themselves with the source, which tends to require that the token maintain a pointer or reference to the source. With get_token being const qualified, the source can only pass a constant reference or pointer to itself to be stored by the token, but the callback almost certainly needs to mutate the source to register itself.

libunifex doesn't have this issue because their implementation of get_token is not const qualified. stdexec follows the specification and had to resort to marking its list of callbacks as mutable, which is usually considered a code smell by many.

I am curious about the reasoning behind this decision. I know that std::stop_source made the same choice, but those implementations do not require callback registration.

ericniebler commented 10 months ago

Comment by ericniebler Thursday Dec 07, 2023 at 19:22 GMT

This is a good question for @lewissbaker

lewissbaker commented 10 months ago

The get_token() method on the in_place_stop_source is const because the operation does not logically modify the visible state of the stop-source, either immediately or allow subsequent modification through the returned stop-token. The only operation that can affect the visible state is a call to request_stop() which is non-const.

Yes, this means you will probably need to use one or more mutable members in the stop-source class to manage the registered stop-callbacks. I don't see this as a problem - that's what the mutable keyword is for - implementing logically const operations that do actually need to mutate the state of the object.

jensmaurer commented 10 months ago

I'd like to point out that "const" member functions are thread-safe per the standard library rules. Can multiple threads safely call get_token() concurrently?

lewissbaker commented 10 months ago

Yes, it is safe to call all member functions of in_place_stop_source concurrently (except for the destructor).

In practice, get_token() just captures a pointer to some internal state of the stop source and does not perform any mutations. The mutations that are performed through the token occur when constructing a stop callback, which is safe to do concurrently, provided the return from the stop callback destructors happen before the associated stop source destructor is called.