2009-06-12 26 views
3

バックグラウンドプロセスを使用したレスポンシブGUIへのアプローチは正しいですか?そうでない場合は、批評して改善をお願いします。特に、どのコードがデッドロックや競合状態に陥る可能性があるかを示します。C#スレッディングとWindowsフォーム

ワーカースレッドをキャンセルして進行状況を報告できる必要があります。私が見たすべての例では、別のオブジェクトではなく、フォーム自体にプロセスコードがあるため、私はBackgroundWorkerを使用しませんでした。私はBackgroundWorkerのLongRunningProcessを継承することを考えましたが、オブジェクト上に不要なメソッドを導入すると考えました。理想的には、私はプロセス( "_lrp")へのフォーム参照を持っていないほうがいいですが、フラグをチェックするLRP上にイベントがない限り、プロセスをキャンセルすることはできません不必要に複雑で、おそらくは間違っているように見えます。

Windowsフォーム(編集:移動* .EndInvokeは、コールバックへの呼び出し)

public partial class MainForm : Form 
{ 
    MethodInvoker _startInvoker = null; 
    MethodInvoker _stopInvoker = null; 
    bool _started = false; 

    LongRunningProcess _lrp = null; 

    private void btnAction_Click(object sender, EventArgs e) 
    { 
     // This button acts as a Start/Stop switch. 
     // GUI handling (changing button text etc) omitted 
     if (!_started) 
     { 
      _started = true; 
      var lrp = new LongRunningProcess(); 

      _startInvoker = new MethodInvoker((Action)(() => Start(lrp))); 
      _startInvoker.BeginInvoke(new AsyncCallback(TransferEnded), null); 
     } 
     else 
     { 
      _started = false; 
      _stopInvoker = new MethodInvoker(Stop); 
       _stopInvoker.BeginInvoke(Stopped, null); 
     } 
    } 

    private void Start(LongRunningProcess lrp) 
    { 
     // Store a reference to the process 
     _lrp = lrp; 

     // This is the same technique used by BackgroundWorker 
     // The long running process calls this event when it 
     // reports its progress 
     _lrp.ProgressChanged += new ProgressChangedEventHandler(_lrp_ProgressChanged); 
     _lrp.RunProcess(); 
    } 

    private void Stop() 
    { 
     // When this flag is set, the LRP will stop processing 
     _lrp.CancellationPending = true; 
    } 

    // This method is called when the process completes 
    private void TransferEnded(IAsyncResult asyncResult) 
    { 
     if (this.InvokeRequired) 
     { 
      this.BeginInvoke(new Action<IAsyncResult>(TransferEnded), asyncResult); 
     } 
     else 
     { 
      _startInvoker.EndInvoke(asyncResult); 
      _started = false; 
      _lrp = null; 
     } 
    } 

    private void Stopped(IAsyncResult asyncResult) 
    { 
     if (this.InvokeRequired) 
     { 
      this.BeginInvoke(new Action<IAsyncResult>(Stopped), asyncResult); 
     } 
     else 
     { 
      _stopInvoker.EndInvoke(asyncResult); 
      _lrp = null; 
     } 
    } 

    private void _lrp_ProgressChanged(object sender, ProgressChangedEventArgs e) 
    { 
     // Update the progress 
     // if (progressBar.InvokeRequired) etc... 
    } 
} 

バックグラウンドプロセス:

public class LongRunningProcess 
{ 
    SendOrPostCallback _progressReporter; 
    private readonly object _syncObject = new object(); 
    private bool _cancellationPending = false; 

    public event ProgressChangedEventHandler ProgressChanged; 

    public bool CancellationPending 
    { 
     get { lock (_syncObject) { return _cancellationPending; } } 
     set { lock (_syncObject) { _cancellationPending = value; } } 
    } 

    private void ReportProgress(int percentProgress) 
    { 
     this._progressReporter(new ProgressChangedEventArgs(percentProgress, null)); 
    } 

    private void ProgressReporter(object arg) 
    { 
     this.OnProgressChanged((ProgressChangedEventArgs)arg); 
    } 

    protected virtual void OnProgressChanged(ProgressChangedEventArgs e) 
    { 
     if (ProgressChanged != null) 
      ProgressChanged(this, e); 
    } 

    public bool RunProcess(string data) 
    { 
     // This code should be in the constructor 
     _progressReporter = new SendOrPostCallback(this.ProgressReporter); 

     for (int i = 0; i < LARGE_NUMBER; ++i) 
     { 
      if (this.CancellationPending) 
       break; 

      // Do work.... 
      // ... 
      // ... 

      // Update progress 
      this.ReportProgress(percentageComplete); 

      // Allow other threads to run 
      Thread.Sleep(0) 
     } 

     return true; 
    } 
} 
+0

"Please critique"は質問とよく似ていないので、閉じるように投票しました。 –

答えて

1

私は別のオブジェクト内のバックグラウンド・プロセスの分離が好き。ただし、同じボタンハンドラでBeginInvokeとEndInvokeを呼び出すため、バックグラウンドプロセスが完了するまでUIスレッドがブロックされています。

MethodInvoker methodInvoker = new MethodInvoker((Action)(() => Start(lrp))); 
IAsyncResult result = methodInvoker.BeginInvoke(new AsyncCallback(TransferEnded), null); 
methodInvoker.EndInvoke(result); 

何か不足していますか?

+0

はい、あなたはこれについて正しいと思います。どのように私は最初にそれを逃したのか分からないが、私はコードをコールバックメソッドに移動します。 – ilitirit

1

私は、あなたのMethodInvoker.BeginInvoke()の使用によってちょっと混乱します。新しいスレッドを作成し、Thread.Start()を使用する代わりに、これを使用する理由がありますか?

BeginInvokeと同じスレッドでEndInvokeを呼び出すため、UIスレッドをブロックする可能性があります。通常のパターンは、受信スレッドでEndInvokeを呼び出すことです。これは非同期I/O操作では確かに当てはまります。ここで適用されない場合は謝罪してください。とにかくLRPが完了するまでUIスレッドがブロックされているかどうかを簡単に判断できるはずです。

最後に、BeginInvokeの副作用を利用して、マネージスレッドプールからワーカースレッドでLRPを開始します。繰り返しますが、これがあなたの意図であることを確認する必要があります。スレッドプールにはキューイングのセマンティクスが含まれており、多数の短命プロセスがロードされると大きな仕事をします。私はそれが長期的なプロセスのための良い選択であるとは確信していません。 Threadクラスを使用して長期実行スレッドを開始することをお勧めします。

また、私はLRPがそれをキャンセルするように通知するあなたの方法は、通常、その目的のためにManualResetEventを使用すると思います。状態を確認するためにイベントをロックすることについて心配する必要はありません。

+0

正直言って、私はThread.Startを使用していない理由があることを知っています、私はそれが今何かを覚えていないことができます。その理由はもう適用されない可能性があるので、私は別の実装を試し、結果を比較します。 – ilitirit

1

あなたは_cancellationPendingを揮発性にし、ロックを回避することができます。 なぜ別のスレッドでStopを呼びますか?あなたは競合状態を避けるために、メソッドを呼び出して、あなたのイベントを変更する必要があり

protected virtual void OnProgressChanged(ProgressChangedEventArgs e) 
{ 
    var progressChanged = ProgressChanged; 
    if (progressChanged != null) 
     progressChanged(this, e); 
} 

背景労働者が収まる場合は、あなたがそれを再コーディングする必要はありません。)

+0

停止時(コールバック)を知る必要があるので、私は別のスレッドでStopを呼び出しますが、このメソッドから離れてProcessCompletedEventなどを使用します。私はBackgroundWorkerを使うことができますが、Workerメソッドをbw.DoWorkイベントと互換性があるように再コード化する必要があります。いくつかの可能性を試してみます。 – ilitirit

+0

私は問題があると思う...あなたの停止方法/コールバックはあなたのlrpの終わりを待たない。しかし、あなたがそれを変更しているなら、それは大丈夫です。 労働者を使用すると、自家製のデザインよりも安全です。 –

0

ギョームが掲載され、あなたがAを持っていますOnProgressChangedメソッドの競合状態では、私は答えが解決策であるとは思わない。まだそれを処理するには、同期オブジェクトが必要です。

private static object eventSyncLock = new object(); 

protected virtual void OnProgressChanged(ProgressChangedEventArgs e) 
{ 
    ProgressChangedEventHandler handler; 
    lock(eventSyncLock) 
    { 
     handler = ProgressChanged; 
    } 
    if (handler != null) 
     handler(this, e); 
} 
+0

私がOnProgressChangedのために使用するコードは、BackgroundWorkerクラスから多かれ少なかれ入っていて、レースコンディションをチェックしません。たぶん私は何かが欠けている。 – ilitirit

+0

私のコードは大丈夫です。ロックは必要ありません。 http://blogs.msdn.com/ericlippert/archive/2009/04/29/events-and-races.aspx –

+0

あなたが掲示したリンクが「このコードは競合状態があり、結果として不正な動作をする」というように、競合状態になります。私のコードは、ハンドラに代入するときに最新の値を得ることを保証します。 http://www.yoda.arachsys.com/csharp/events.html – scottm

0

BackgroundWorkerを使用して、作業クラスをFormクラスの外に移動することもできます。あなたのクラスの労働者は、その方法で仕事をしてください。 WorkにパラメータとしてBackgroundWorkerを渡し、最初のメソッドにnullを送るBackgroundWorker以外のシグネチャをWorkメソッドにオーバーロードさせます。フォームで次に

、ProgressReportingを持っているのBackgroundWorkerを使用して、あなたの仕事で(BackgroundWorkerのbgWorker、のparams [] otherParamsオブジェクト)は、次のようなステートメントを含めることができます。

if(bgWorker != null && bgWorker.WorkerReportsProgress) 
    { 
     bgWorker.ReportProgress(percentage); 
    } 

を...と同様にCancellationPendingの小切手が含まれています。

次に、フォームコードでイベントを処理できます。最初にbgWorker.DoWork += new DoWorkEventHandler(startBgWorker);を設定します。このメソッドは、Worker.Workメソッドを起動し、bgWorkerを引数として渡します。

これは、bgWorker.RunWorkerAsyncというボタンイベントからキックオフすることができます。

2番目の取り消しボタンでbgWorker.CancelAsyncを呼び出すと、CancellationPendingを確認したセクションにキャッチされます。

成功またはキャンセルすると、RunWorkerCompletedイベントが処理され、ワー​​カーがキャンセルされたかどうかがチェックされます。それからそれが成功したと仮定しなかったなら、そのルートに行く。

Workメソッドをオーバーロードすると、FormsまたはComponentModelを気にしないコードから再利用できるようになります。

そしてもちろん、その上にホイールを再構築する必要なく、progresschangedイベントを実装します。 ProTip:ProgressChangedEventArgsはintをとりますが、最大100にはなりません。より細かい進捗率を報告するには、乗数(100など)の引数を渡します。したがって、14.32%は1432の進捗になります。表示形式を設定するか、プログレスバーを上書きするか、テキストフィールドとして表示します。 (すべてDRYフレンドリーなデザイン)

関連する問題