【C#】非同期メソッドの作り方について悩んでいます。

Question

##前提
・VS2019
・Windows Forms で実行していると仮定

##質問したい事
ライブラリの一部に、戻り値にTaskを持ったメソッドを作ろうとしています。
どのように非同期メソッドを作るのが正しいのかが知りたいです。

##悩んでいる事
ライブラリ内のメソッドで、Task.Run を使ってタスクを開始してもよいものなのでしょうか。

気にしている事として、幾つかのサイトをみてみた所Task.Run(or start, Task.FactoryStart)と書いてタスクを開始するのはユーザーが決めることでありライブラリが決めることではない、という文言が書いてありライブラリが勝手にスレッドプールを埋めてしまわないように配慮するのは確かにその通りなのかなと感じました。
https://www.infoq.com/jp/articles/Async-API-Design/

ただ、ライブラリのメソッド内でTask.Runを書かないという制約を守りつつ非同期メソッドを作るにはどうするのが正しい書き方なのかがわかりません。

##具体的な例
非常に簡易的ですが、以下のような非同期メソッドをライブラリに組み込もうとするとします。

public static Task<object> HogeAsync()
{
    var tcs = new TaskCompletionSource<object>();
    new Task<object>(() =>
    {
        //something
        return "hoge";
    }).ContinueWith(t =>
    {
        try
        {
            if (t.IsFaulted)
            {
                tcs.TrySetException(t.Exception);
            }
            else if (t.IsCanceled)
            {
                tcs.TrySetCanceled();
            }
            else
            {
                tcs.TrySetResult(t.Result);
            }
        }
        catch (Exception e)
        {
            tcs.TrySetException(e);
        }
    });

    return tcs.Task;
}

これを呼び出す際には、以下の様になります。

var hoge = await HogeAsync();

しかし、これを Windows Forms 上で実行するととトークンをキャンセルするまで。処理がどこかへ行ってしまいます。
処理がどこかへいってしまう理由として、Taskが開始されていない状態でTaskを実行しようとしてしまっているせいだと考えています。なので先ほどのメソッドを以下の様に書き換えれば思った通りの挙動をしてくれます。

public static Task<object> HogeAsync()
{
    var tcs = new TaskCompletionSource<object>();
    //この部分をTask.Runにしてタスクを開始する
    Task.Run<object>(() =>

しかしこうすると、ライブラリのメソッド内でTask.Runを書かないという制約を守れなくなります。
一体、どうすれば…。

##ほかに調べた事
ネイティブなメソッドでは、なぜawaitキーワードで上手く待てるのかを調査してみました。
オーソドックスに、HttpClientクラスのGetAsyncメソッドを追っていってみたところ、Task.Runが使われている箇所がありました。使ってもよい場面があるのでしょうか。。

try
{
  HttpWebRequest prepareWebRequest = this.CreateAndPrepareWebRequest(request);
  state.webRequest = prepareWebRequest;
  cancellationToken.Register(HttpClientHandler.s_onCancel, (object) prepareWebRequest);
  if (ExecutionContext.IsFlowSuppressed())
  {
    IWebProxy webProxy = (IWebProxy) null;
    if (this._useProxy)
      webProxy = this._proxy ?? WebRequest.DefaultWebProxy;
    if (this.UseDefaultCredentials || this.Credentials != null || webProxy != null && webProxy.Credentials != null)
      this.SafeCaptureIdenity(state);
  }
  //ここでTask.Runつかってるじゃん
  Task.Run((Action) (() => this._startRequest((object) state)));
}
catch (Exception ex)
{
  this.HandleAsyncException(state, ex);
}

Comments

[gentaro]

> オーソドックスに、HttpClientクラスのGetAsyncメソッドを追っていってみたところ、Task.Runが使われている箇所

たぶんこれSendAsyncだと思いますが…。（検索してみたらまったく同じコードが引っかかったため）
https://github.com/microsoft/referencesource/blob/e0bf122d0e52a42688b92bb4be2cfd66ca3c2f07/System/net/System/Net/Http/HttpClientHandler.cs#L871

コメント見たらヘルパータスクが云々…って書いてあるから、本筋の処理じゃないからOKって意味なんだろか？（全体像まで追いきるのはしんどいそうなので諦めましたが）

[OXamarin]

指摘ありがとうございます。
確かに、Task.Runが記述されているのはSendAsync内ですね。

ステップ実行をして確認したんですが、GetAsync内の
base.SendAsync(request, linkedCts.Token).ContinueWithStandard<HttpResponseMessage>((Action<Task<HttpResponseMessage>>) (task =>

から呼ばれていましたので、「GetAsyncメソッドを追ったら」という表現をしていました。

Answer 1

こんにちは。

これは根本的に勘違いしている部分があって、「本質的に非同期でない処理を Task として提供するな」というだけの話なんですね。
内部に非同期的な処理が含まれるなら、それを実行するメソッドも非同期になるし、非同期的な処理が含まれないなら、それは同期メソッドになるべきなのです。
その同期メソッドを非同期実行するかどうかはユーザが勝手に選べ、というのがライブラリ API 設計のベストプラクティスだと言う意味です。
「戻り値にTaskを持ったメソッドを作ろうとしています。」というのがそもそも見当違いなのです。戻り値の Task は「そうせざるを得ない」でなるものです。

質問のメソッドを見てみましたが、Task.Run で包もうとしているのが同期処理なのであれば、「その同期処理をそのまま提供してください。」
非同期処理なら「非同期メソッドにして、Task.Run は使わないでください。」
TaskCompletionSource は、イベント処理などの非同期スケジュールを実装しなければならない場合に使うものなので、質問のコードの場合ではおそらく不適当です。

もう少し詳細な要件や事情に踏み込んだサンプルがあれば、ピンポイントな話ができると思います。

Comments

[Zuishin]

戻り値に Task を返すメソッドを提供すること自体は問題ありません。Task を返すメソッドはメソッド名に Async を入れるという規約があります。
このコードの問題はそこではなく、Task が二つあることだと思います。

[tamoto]

はい、最初からそう言っています。非同期でない処理を Task として提供することが間違いです。

コードの問題というのはスタックする件でしょうか？ Task が2つあることは関係なく、`new Task()` で作っているので実行されてないだけです。

[Zuishin]

返してるのが TaskCompletionSource の Task で、作った方の Task を捨てているからでしょう。二つあるから一つ忘れているという意味です。非同期でない処理ではなく、非同期処理の中でさらに非同期処理を作っています。

[tamoto]

そうですね。これがナンセンスな構造なのは同意しますが、最初に作った方の Task が稼働していれば TaskCompletionSource の Task もいずれ完了するので、2つあるがために起きている不具合ではないですよ。2つあることに何の意味もないだけです。

[gentaro]

横からすみません。私の疑問が的外れだったら申し訳ないんですか、

> 「戻り値にTaskを持ったメソッドを作ろうとしています。」というのがそもそも見当違いなのです。戻り値の Task は「そうせざるを得ない」でなるものです。

というのは「同期処理（の塊）をTaskでラップした、Taskオブジェクトのファクトリメソッド（そのメソッド自体はasyncキーワードもついておらず、内部でTask.Runもしていない＝ただの同期メソッド）」はNGなんでしょうか？

[Zuishin]

> 「戻り値にTaskを持ったメソッドを作ろうとしています。」というのがそもそも見当違いなのです。

について、そこは見当違いではないという意見です。新たに Task を作って稼働させれば良いという問題ではありません。それを稼働させるのはユーザーです。

Task を二つ作り、どちらも稼働させず片方だけ返しているのが問題ですが、その解決法として「返さない方を稼働させる」「そもそも二つ作らない」の二種類があると思います。「返さない方を稼働させる」のは良い方法とは思えません。一つで済むなら一つだけ作ってそれを返すのが良いと考えます。

[tamoto]

gentaro さんへ
内部では Task.Run もしていない、というのは、「稼働していない Task」を返すということでしょうか？それは非常に危険なのでやっていはいけないものです。C# の非同期フローは、構築された Task が「開始済み」であることを暗黙的に要求しています。
「同期処理の塊を Task でラップ」は、ラップしないでその同期処理自体を返すほうがライブラリの API 設計として親切だ、という話です。

[tamoto]

Zuishin さんへ
中身が全部同期処理なのに、それをあえて Task.Run で包んで Task として返してあるのが良いと思いますか？
そうは思いません。
単なる同期メソッドであれば、ユーザは同期で処理することもできますし、それを非同期で実行することもできます。選択権はユーザが持つべきでしょう。

後半の、Task 一つか二つかの二択なら、一つだけ作って返すほうが明確に良いというのは完全に同意します。無意味に複雑にする理由は全くありません。

[gentaro]

なるほど。
TaskCompletionSourceを使うことがどうかはとりあえず置いておくと、Zuishinさんは「Taskを作って返す」行為そのものはOKという見解で、tamotoさんは「NG」という見解だと理解しましたが、そこはOKですか？
であれば、tamotoさんはnew Task()そのものが危険だと仰っているように見えるため、私はZuishinさんの仰っている事が正しいように見えますが…。
Taskはあくまで「オブジェクト」であって、それを作って返す行為自体が悪いようにはちょっと思えません。（そのTaskを起動するかどうかの選択肢はユーザー側にあるので）

[Zuishin]

同期処理しかしないなら Task を返す必要が無いことに同意しますが、これは一例にすぎず、非同期が必要という前提だと思います。

[tamoto]

gentaro さんへ
標準ライブラリやサードーパーティライブラリに Task を非稼働で返す API はおそらく存在しません。それは async/await の言語仕様的に想定されていません。当然非同期メソッドも開始した状態の Task を返します。

Zuishin さんへ
非同期が必要 (内部で非同期処理を行う) なら、「同期処理をラップ」というのがそもそも間違いで、最初から非同期メソッドを使えば良い話でしょう。Task.Run はそういう意味で、ユーザサイドで利用されるメソッドだ、ということです。
ただし、仮に何らかのフレームワークを設計していて、当該 API が明示的にスレッドプールを可動させることを目的としている場合には Task.Run が利用されることはありますね。なので問答無用で禁止というわけではないです。

[gentaro]

すみません。なんか前提のところからの話になるんですが…。
「非同期メソッド」てasyncキーワードが付いているものだと認識しているので、そもそも質問者のコードはそうなっておらず、「同期メソッド」だと思っているんですが、あってます？
「内部で同期処理しかしない非同期メソッド」がおかしい（同期メソッドでいいじゃん）、というのは完全に同意します。

[Zuishin]

ラップしていいかどうかではなく、起動していいかどうかという質問だと思います。起動せざるを得なくなったのは、捨てられているからで、問題の本質はそこにあるのではないですか？ ラップしているのは単に例となるソースを単純にするためだと思いますが。

gentaro さん、非同期に async は必要ありませんよ。これは内部で await を使うソースをコンパイルする時のみ必要となるキーワードです。

[tamoto]

gentaro さんへ
そうですね。正しいです。
メソッドそのものは同期で、中では Task を作っており、その Task は同期処理をラップしただけ、というものになっています。
そこで、Task がラップしている「同期処理」をそのまま返すのがライブラリの API としてのベストプラクティスです。

[tamoto]

Zuishin さんへ
その「問題」というのはスタックする件でいいんですよね？それはラップやら二重やら捨てられている以前にただ「起動していないから」で、Task を起動していないこと自体が非同期 API の設計から反しています。
仮に TaskCompletionSource を使わずに new した Task をそのまま返したところでスタックすることには変わりありませんよ。

[gentaro]

Zuishinさん
「非同期"処理"にasyncは不要」という趣旨だと思いますが、.NETで「非同期メソッド」というと
https://ufcpp.net/study/csharp/sp5_async.html#async
この定義だと思って話を追っていたので、なんか混乱しました。

[gentaro]

tamotoさん
なんとなく趣旨は理解できた気がします。

[gentaro]

なんか話を掻き乱しただけのような気がしてきて申し訳ない。

> 「戻り値にTaskを持ったメソッドを作ろうとしています。」というのがそもそも見当違いなのです。戻り値の Task は「そうせざるを得ない」でなるものです。

この部分は「『非同期メソッド』を作るなら、当然内部で非同期処理を実行しているはずで、投げっぱなしにするのではなくちゃんとTaskを返す必要があるため、必然的にそうなる」という意味ですね。

一文だけ切り取って読んでしまい、ちょっと混乱してました。すみません。

[tamoto]

gentaro さんへ
主張と相違ないです。そこがちゃんと伝わったのならよかったです。

[Zuishin]

tamoto さん
スタックというより、起動しなければ動かないので、誰も起動しなければ止まったままだと思います。私が問題にしているのは、「タスクが二つ動いている」ということで、「TaskCompletionSource を使うな」というのは「タスクを一つにまとめたら」という意味です。そのまま削除することではありません。
同期処理なら Task を返す必要はないことに同意しますし、もともと非同期ならその Task を返すことに同意します。その Task を返すということは、TaskCompletionSource を使わないということです。
しかし
> 「戻り値にTaskを持ったメソッドを作ろうとしています。」というのがそもそも見当違いなのです。
これには同意できません。

gentaro さん
次のコードは DoSomething が非同期メソッドですが、async は使われていません。内部に await が無いからです。しかし Button1_Clilck には async が使われています。await があるからです。
そして非同期なので Task を返しています。これを void にすると動きません。
await キーワードは非同期に適したメソッドの特殊なコンパイルをするためのキーワードですが、これはもともと C# には無かったもので、以前は await という変数を作ることができました。その互換性を保つため、「このメソッド内部で使われている await はキーワードですよ」という印に async を使います。

using System;
using System.Diagnostics;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WindowsFormsApp1
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}

Task DoSomething()
{
return Task.Run(() =>
{
for (int i = 0; i < 100; i++)
{
Debug.WriteLine(i);
}
});
}

private async void Button1_Click(object sender, EventArgs e)
{
await DoSomething();
Debug.WriteLine("Finished");
}
}
}

[Zuishin]

> 以前は await という変数を作ることができました。

失礼しました。async を使わないメソッド内部では今でも作ることができます。

[tamoto]

Zuishin さんへ
誤解させたかもしれません。
見当違いというのは、「戻り値にTaskを持ったメソッドを作ろうとしています。」というのを「(中身が同期処理のみで構成されたものを) 戻り値を Task にしたメソッドを作ろうとしています。」と読み取ったことに拠ります。
「本質的に非同期」な処理を行うなら戻り値が Task になるのは当然です。
もしもそのような解釈で読み取っていたのでしたら、こちらの言葉が足りていませんでした。申し訳ないです。

また、上2項には全面的に同意します。

[gentaro]

Zuishinさん
サンプルまで提示していただいて申し訳ない。

言葉の定義の問題で、Zuishinさんは「async/awaitが登場する前」からの歴史的経緯を踏まえて「広義の非同期メソッド」を前提にされていたと思いますが、私は上記のとおり「asyncキーワードがついたメソッド」という「狭義の非同期メソッド」を想定していたため、ちょっと齟齬があったようです。
私の疑問は解消できました。

[Zuishin]

なるほど。誤解があったかもしれません。読み直してみます。

[OXamarin]

tamotoさん
Zuishinさん
gentaroさん

回答と多くの補足説明をありがとうございます。
ここまで多くのレスがつくとは思っておらず、非常にびっくりしました。

>「戻り値にTaskを持ったメソッドを作ろうとしています。」というのを「(中身が同期処理のみで構成されたものを) 戻り値を Task にしたメソッドを作ろうとしています。」と読み取ったことに拠ります。
私がやろうとしていたことはこの通りでございます。

ここまでの回答をまとめさせて頂きますと、
・同期処理しかしないのに Task として提供するな。
・なるべくしてTaskを返すメソッドとなるはず（メソッド内で非同期処理が呼ばれている）
・C# の非同期フローは、構築された Task が「開始済み」であることを暗黙的に要求している

つまり、私が実装しようとしていた事自体が非同期ライブラリのポリシーに反していたということですね。作りながら違和感を覚えていたので、根本的な間違いに気付けてよかったです。ありがとうございました！

[gentaro]

私も途中混乱してしまいましたが、考えを整理するのに役立ちました。ありがとうございます。

中身が非同期処理なのであれば当然の結果としてTaskが戻り値になるし、同期処理だけをラップしたTaskの(つまりTask.Run()の結果を返す)場合は、目的はおそらく非ブロッキング処理にしたい、とか並列実行したい、というものでしょうけど、それは利用者側の責務であってライブラリでやるべきじゃない、という事ですね。元記事の内容もなんとなく理解できた気がします。

Answer 2

参考にされている記事の意味がよくわかっていませんので外れかもしれませんが・・・

Task.Run を使わない => ライブラリの利用者に非同期を強要しない・・・という意味ではなかろうかと思います。

その理解で正しければ、どうすれば良いかは、同期・非同期を選択できる余地を与えると言うことになるかと思います。

具体的には、例えば、

(1) 同期版・非同期版の両方のメソッドを実装する、または

(2) 同期版のみ実装してユーザーが非同期にしたい場合は、ユーザーに Task.Run を使ってもらう。

・・・で、少なくとも記事に書いてあった懸念はなくなると思うのですが。

HttpClient には非同期版しか実装してないようなのが解せませんが (隠し拡張メソッドの同期版とかがある？ 参考にされている記事は 6 年も前のものなので、状況が変わった？)

以上、個人的意見です。外れでしたらすみません。

Comments

[gentaro]

この記事、直訳っぽくてぱっと読んだだけでは頭に入ってこないのでなかなか理解できていないですけど、少なくとも(1)は記事の内容と矛盾しません？

> 責任あるライブラリ開発者になる必要があります。メソッドが本当に同期処理を行うなら、非同期バージョンを提供せずに、同期バージョンだけを提供します。同様に本当に非同期だったら、非同期バージョンだけを提供します。

まぁ非同期版のメソッドをWaitでラップした同期版を作るな、という文脈ではありますが…。

[退会済みユーザー]

> 少なくとも(1)は記事の内容と矛盾しません？

どうでしょう？　

記事の“ライブラリ内でTask.Runを使わない”のセクションに書いてあるのは、非同期メソッドを使うと (a) スレッドプールからスレッドを取得するのに時間がかかるという問題がある、(b) スレッドプールの枯渇の問題がある・・・と言う 2 点です。

自分はここだけ見て回答に書きました、Task.Run を使わない => ライブラリの利用者に非同期を強要しない・・・という意味ではなかろうかと思ったわけです。

そういう意味にとらえれば矛盾はしないと思いますけど。

(a), (b) の問題点を知らないライブラリの利用者が、使うべきでないのに深く考えずに（マルチスレッドで処理が早くなるというような勘違いをして）非同期版を使ってしまうという話はあるかもしれませんが、それはまた違う問題でしょう。

実際、IO バウンドを行う非同期版・同期版メソッド両方を実装した Microsoft のライブラリはありますし、サービス参照で非同期版・同期版のメソッド両方のメソッドが自動生成されます。

Answer 3

TaskCompletionSource を使うからでは？