2017-11-07 3 views
0
public class ThreadingIssue 
{ 
    int accum; 

    public void Squaring(int x) 
    { 
     // Creates a random pause to check for thread 
     int pauseFor = rnd.Next(1, 10); 
     Thread.Sleep(pauseFor); 

     accum += x*x; 

    } 

    public void DoSquaring() 
    { 
     accum = 0; 
     for (int i = 1; i <= 100; i++) 
     { 
      threads.Add(new Thread(() => Squaring(i))); 
      threads[i - 1].Start(); 
     } 
     } 

本質的に私は答えとして毎回338350を得たいと思っていましたが、明らかにスレッドの問題がありません。そのようACCUM変数をロックし、その後C#基本的なマルチスレッドの問題 - 競合状態を解決するためにロックを使用する方法

private System.Object lockThis = new System.Object(); 

と::

lock (lockThis) 
{ 
    accum += x*x; 
} 

しかし、これで問題が解決しない私のようなオブジェクトを作成しようとしました。私は何が間違っている/間違っていますか?

+0

なぜ配列内のスレッドへの参照を保持していますか? – Enigmativity

答えて

7

forループに問題があります。

for (int i = 1; i <= 100; i++) 
{ 
    threads.Add(new Thread(() => Squaring(i))); 
    threads[i - 1].Start(); 
} 

ループ変数iは各スレッドを起動するときラムダ() => Squaring(i)内で使用されているが、時間によって任意のスレッドが作成されたループが終了したとiの値が101あります。

これを修正するには、ループ内にiのコピーをキャプチャして使用する必要があります。

これを試してみてください:

for (int i = 1; i <= 100; i++) 
{ 
    int j = i; 
    threads.Add(new Thread(() => Squaring(j))); 
    threads[i - 1].Start(); 
} 

はまたaccum += x * x;線を中心lockする必要があります。

private object gate = new object(); 
public void Squaring(int x) 
{ 
    // Creates a random pause to check for thread 
    int pauseFor = rnd.Next(1, 10); 
    Thread.Sleep(pauseFor); 

    lock (gate) 
    { 
     accum += x * x; 
    } 
} 
1

まず、次の操作を行います。

public class ThreadingIssue 
{ 
    int accum; 
    private readonly object _syncLock = new object(); 

    public void Squaring(int x) 
    { 
     // Creates a random pause to check for thread 
     int pauseFor = rnd.Next(1, 10); 
     Thread.Sleep(pauseFor); 

     lock(_syncLock) 
     { 
      accum += x*x; 
     } 
    } 

    public void DoSquaring() 
    { 
     accum = 0; 
     for (int i = 1; i <= 100; i++) 
     { 
      int number = i; 
      threads.Add(new Thread(() => Squaring(number))); 
      threads[i - 1].Start(); 
     } 
     } 
    } 
2

を、私はこれを頼まれたものを、厳密ではないですけど、正しい方向にステアリングあなたの利益のために、この作業は非常に最小限に最適ですLINQのとTasks.Parallel:

var bag = new ConcurrentBag<KeyValuePair<int, int>>(); 

Parallel.For(1, 101, 
    i => 
    { 
     bag.Add(new KeyValuePair<int, int>(i, i * i)); 
    }); 

var squares = bag.ToDictionary(pair => pair.Key, pair => pair.Value); 

Console.WriteLine("Square of 56 is " + squares[56].ToString()); 
Console.WriteLine("Sum of all squares is " + squares.Sum(pair => pair.Value).ToString()); 

これは完全に手動でロックを管理するための必要性を削除し、また、0123のために比較的新しいSystem.Collections.Concurrent名前空間を利用していますワークスペースを並列化するための名前空間。ループは多くのスレッドを使用してコレクションを埋め、OSがシステムを応答し続けるために生成するスレッドの数をOSに決定させ、辞書にキャストされた後に必要な処理を実行できます。また、ParallelOptionsインスタンスをParallel.For()コールに渡して、タスクの実行時間が短いか長いかを指定して、OSに優先順位の設定方法を伝え、タスクの最大並列度を指定することもできます。

フォームのプライマリスレッドでTasks.Parallelを使用しないでください。

関連する問題