cpprefjp / site

cpprefjpサイトのMarkdownソース
https://cpprefjp.github.io/
368 stars 152 forks source link

promiseのデストラクタが例外オブジェクトを格納することの記述がない #1270

Closed tshino closed 1 month ago

tshino commented 2 months ago

std::promiseのデストラクタは broken_promise というエラー定数を持つ std::future_error /例外を投げる/例外オブジェクトを共有状態に格納する/場合があることが、 https://en.cppreference.com/w/cpp/thread/promise/%7Epromise には書いてありますが、本サイトではとくに記載がありませんでした。 https://cpprefjp.github.io/reference/future/promise/op_destructor.html

自分は規格の記載をまだ調べ切れていないのですが、/少なくともC++11時点では例外の記述がなさそうでした/C++11相当とされるN3337にcppreference.comと同様の記載がありました/。

手元のコンパイラ(C++17)でstd::promiseを作って、そのまま破棄すると/例外が投げられる/対応するstd::future::get()で例外が投げられる/ことは確認出来ました。

問題点の指摘のみですみませんが、よろしくお願いします。

tshino commented 2 months ago

すみません、少し調べ直していますが私の認識に不正確なところがありました。少しお待ちください。

tshino commented 2 months ago

std::promiseのデストラクタが例外を投げるというのは私の誤解でした。 cppreference.com に書いてあったのは、ある条件のとき「例外を投げる」ではなく「例外オブジェクトを共有状態に格納する」でした。

また、N3337(C++11相当とのこと)に書かれていないというのも誤解で、同様のことが書かれていました。デストラクタのEffectsには「共有状態を放棄する(Abandons any sahred state)」とだけ書かれていたため誤解しましたが、別途 30.6.4 Shared state に記述があり「共有状態を放棄するとは、~」として上記の動作を含む具体的な効果が書かれています。

さらに、手元で例外が投げられることを確認したのも誤解だったようで、std::promiseのデストラクタが上記の動作をした場合に、std::future::get() が例外を投げるようでした。

結論として、

でした。 問題点は、

です。 少なくとも std::promise のデストラクタの効果の欄に記載を追加すれば良さそうです。

faithandbrave commented 2 months ago

ご指摘と調査ありがとうございます。 指摘内容と規格を確認しました。

最初にご指摘のあったように、共有状態を破棄する際に、broken_promiseをもつfuture_errorが共有状態に書き込まれるようですね。

こちら、 @tshino さんの方で修正できますでしょうか?

tshino commented 2 months ago

確認ありがとうございます。 やってみます。

tshino commented 2 months ago

promiseデストラクタは修正しましたが、同じ問題が他のメソッドにもあるので調べて同様に修正します。

tshino commented 2 months ago

Effectsに(Note内をのぞいて)"abandon (any shared state)" が含まれるメソッドは3つありました。(N3337 および N4950 で同じ、以下N3337を引用)

ここで問題を発見しました。 std::packaged_task のムーブ代入演算子をみると、

Effects:
- releases any shared state (30.6.4).
- packaged_task(std::move(rhs)).swap(*this).

なので、abandonせず(abandon処理の一部である)releaseのみ行います。

一方で、std::packaged_taskvoid reset() の効果では

Effects: as if *this = packaged_task(std::move(f)), where f is the task stored in *this. [ Note:
This constructs a new shared state for *this. The old state is abandoned (30.6.4). — end note ]

のように書かれ、as-if として古い共有状態に関してはムーブ代入演算子と等価であると読めるにも関わらず、Note内では abandon することが言及されています。

ひとまず、明示的にabandonと書かれているメソッドのページは更新しておきますが、 問題点についてはどうしましょうか。

問題の箇所の現在の記述はEffects本文とNoteを合わせた記述になっていて、ムーブ代入演算子には「解放(releaseの訳だと思う)」、resetには「放棄(abandonの訳だと思う)」と書かれています。

Noteを信じる場合、少なくともresetの記事には、本チケットの趣旨に沿ってbroken_promiseまで踏み込んで記載することになります。 Noteを(信じると矛盾するので)信じない場合、Noteから来ている「放棄」の記述を削除し、as-ifに従って「解放」と直せば規格の文字通りになりますね。

faithandbrave commented 2 months ago

ムーブ代入演算子は共有状態がほかのpackaged_taskオブジェクトに移りますが、reset()では共有状態が放棄されるので、Noteに書かれているようにabandonされるのが正しそうではありますね。

#include <iostream>
#include <future>

int main()
{
  std::packaged_task<int()> task{[]{ return 0; }};
  std::future<int> f = task.get_future();

  task.reset();

  try {
    // 非同期処理の結果値を取得する
    std::cout << f.get() << std::endl;
  }
  catch (std::future_error& e) {
    std::cout << e.what() << std::endl;
  }
}
The associated promise has been destructed prior to the associated state becoming ready.
tshino commented 2 months ago

なるほど。 そのコードの test.reset()task = std::packaged_task<int()>() に置き換えても同じ結果が得られました。 実装では、ムーブ代入演算子に関しても規格にある release の動作ではなく abandon の動作を行っているようですね。

faithandbrave commented 2 months ago

packaged_taskstd::async()内で使われるマイナー機能であるために仕様矛盾が残ってるのかもですね。 cpprefjpとしては、「仕様ではこう書いてあるが実装はこうなっていて、実装が正しいと考える」のように書いておくのがよいかと思います。

yohhoy commented 2 months ago

packaged_taskstd::async()内で使われるマイナー機能であるために仕様矛盾が残ってるのかもですね。

私も上記解釈に賛同です。C++標準ライブラリ仕様バグのような気がします。

Future/Promise関連で "releases any shared state" と言及があるのは共有状態(shared state)を読み取る future/shared_future 側が基本となっており、指摘のpackaged_taskムーブ代入演算子のみが唯一の共有状態書き込み側に区分されます。仮に仕様通りに古い共有状態を abandon せずに release してしまうと共有状態を書き換える経路が失われ、Future側でデッドロックを引き起すことが予想されます。

tshino commented 2 months ago

念のため試したら、どうも Visual C++では様子が違いました。

メソッド Clang GCC Visual C++
packaged_task ムーブ代入演算子 abandon abandon release
packaged_task reset() abandon abandon abandon

releaseと書いたのは、f.get()が帰ってこなかったのでそう判断しました。 実用上は、この動作に依存するコードは避けた方が良いですねぇ。

faithandbrave commented 2 months ago

promiseはともかくpackage_taskを持ち運ぶことはあまりないと思いますが、備考にその情報を書いておくとうれしいかもですね。

tshino commented 1 month ago

記載、完了しました。 確認など色々ありがとうございました。