[C#] 競合防止はフラグだけではない。

こんなコードで問題が起きた。

       static void tt_Tick(object sender, EventArgs s)
        {
            if (aclock == false)
            {
                aclock = true;
                // データが登録されていたら、それをログしていく。
                if (xs.Count > 0)
                {
                    logdata dd = xs[0];
                        try
                        {
                            using (StreamWriter sw = new StreamWriter(dd.filename, true))
                            {
                                // 改行付きで出力
                                sw.WriteLine(dd.dt.ToString("yyyy年MM月dd日 HH時mm分ss秒") + " " + dd.data);
                                sw.Close();
                            }
                        }
                        catch (Exception e)
                        {
                            lasterr = e.Message;
                        }
                    }
                    if (xs.Count > 0)
                    {
                        xs.RemoveAt(0);
                    }
                
                aclock = false;
            }
        }

これはタイマーから約300msごとに起動されているはずなのだけど……。なぜか「xs.CountがゼロなのにRemoveAtしようとした」と怒られて止まってしまった。まぁ、約300msで二日保ったのは大したものだけど。

で、24~27行に注目してほしい。
xsの数を0より大きいと確認しているにもかかわらず、その次の行でxs.RemoveAt(0)が「0しかないものをさらに削除しようとした」という例外になったのだ。
ここまでモロだと逆にわかりやすい。
ライブラリ自体がぶっ壊れているのでないなら、おそらく理由は競合しかないと思うのですよね。
メモリリークの気配もないのに、よくこんなシビアな競合が起きたものだと思う。しかも aclockってboolデータは一応、これでも二重動作防止のつもりらしいのに。きっとI/O絡みで前の仕事が時間かかりすぎ、とても微妙なぶつかりかたをしたのだね。うん、きっとそうだ。

で、対策なのだけど。以下のコンセプトにした。

・そもそも多重で動かないようにしてみる。

具体的には以下を追加したのね。

1.処理に入る前に呼び元のタイマーを一時停止する。これは終了後に再び動かす。
2.それでもすり抜けた場合にさなえ、該当部分をtry-catchで例外を捕まえてみる。
3.消すのは一個ずつなんだから、残り一個ならむしろ clear()させてみるか?
 (かりに軽い競合が起きたとしても、残りゼロにclear()かける方が被害は小さいと思う)
4.今までのboolも残しておく。

んで、対応後のコードは以下に。

        static void tt_Tick(object sender, EventArgs s)
        {
            System.Timers.Timer wt = (System.Timers.Timer)sender; 
            wt.Stop(); // おまえ処理中はちょっと止まれ
            if (aclock == false)
            {
                aclock = true;
                // データが登録されていたら、それをログしていく。
                if (xs.Count > 0)
                {
                    logdata dd = xs[0];
                        try
                        {
                            using (StreamWriter sw = new StreamWriter(dd.filename, true))
                            {
                                // 改行付きで出力
                                sw.WriteLine(dd.dt.ToString("yyyy年MM月dd日 HH時mm分ss秒") + " " + dd.data);
                                sw.Close();
                            }
                        }
                        catch (Exception e)
                        {
                            lasterr = e.Message;
                        }
                    }
                    if (xs.Count > 0)
                    {
                        if (xs.Count > 1)
                        {
                            try
                            {
                                xs.RemoveAt(0);
                            }
                            catch (Exception e)
                            {
                                lasterr = e.Message;
                            }
                        }
                        else
                        {
                            xs.Clear();
                        }
                    }
                
                aclock = false;
            }
            wt.Start(); // よし、動いていいぞ。
        }

「[C#] 競合防止はフラグだけではない。」への1件のフィードバック

  1. ここの調整だけど、結局は呼び出し頻度自体を1/10以下にする事にして、代わりにデータをまとめて出力するようにした。即時性が失われるわけだけど、そこはデータ取得時に日時も記録するという形で。

    くさいものは元から断て?(違)

コメントを残す

メールアドレスが公開されることはありません。 * が付いている欄は必須項目です

日本語が含まれない投稿は無視されますのでご注意ください。(スパム対策)