2011-06-01 32 views
43

以下は、かなり簡単なスレッドセーフなロギングクラスを実装する正しい方法でしょうか?スレッドセーフなロギングクラスの実装

私は明示的にTextWriterを閉じることはできないことを知っています、それは問題でしょうか?私が最初にTextWriter.Synchronizedメソッドを使用する場合、私は静的コンストラクタでそれを初期化し、それが読み取り専用そうのように作られるまで

は、動作していないようでした:

public static class Logger 
{ 
    static readonly TextWriter tw; 

    static Logger() 
    { 
     tw = TextWriter.Synchronized(File.AppendText(SPath() + "\\Log.txt")); 
    } 

    public static string SPath() 
    { 
     return ConfigManager.GetAppSetting("logPath"); 
    } 

    public static void Write(string logMessage) 
    { 
     try 
     { 
      Log(logMessage, tw); 
     } 
     catch (IOException e) 
     { 
      tw.Close(); 
     } 
    } 

    public static void Log(string logMessage, TextWriter w) 
    { 
     w.WriteLine("{0} {1}", DateTime.Now.ToLongTimeString(), 
      DateTime.Now.ToLongDateString()); 
     w.WriteLine(" :"); 
     w.WriteLine(" :{0}", logMessage); 
     w.WriteLine("-------------------------------"); 

     // Update the underlying file. 
     w.Flush(); 
    } 
} 
+5

私はlog4netを見ることをお勧めします。 http://logging.apache.org/log4net/index.html –

+0

第三者ロギングフレームワークは間違いなく良い選択です... –

答えて

83

この上車輪の再発明しないことをお勧めします私たちからの第三者の提案を探しているわけではありません。)

他の人は、スレッドセーフであるTextWriterを作成しています。これは、WriteLineの呼び出しがスレッドセーフであることを示しています。つまり、WriteLineへの呼び出しがアトミック操作として実行されるわけではありません。これにより、4つのWriteLine呼び出しが順番に実行されるという保証はありません。あなたはスレッドセーフTextWriterを持っているかもしれませんが、スレッドセーフなLogger.Logメソッドはありません;)なぜですか?なぜなら、これらの4つの呼び出し中のどの時点でも、別のスレッドがLogも呼び出すことになるからです。これは、あなたのWriteLineコールが同期しないことを意味します。この問題を解決する方法はとても似lock文を使用して、次のとおりです。

private static readonly object _syncObject = new object(); 

public static void Log(string logMessage, TextWriter w) { 
    // only one thread can own this lock, so other threads 
    // entering this method will wait here until lock is 
    // available. 
    lock(_syncObject) { 
     w.WriteLine("{0} {1}", DateTime.Now.ToLongTimeString(), 
      DateTime.Now.ToLongDateString()); 
     w.WriteLine(" :"); 
     w.WriteLine(" :{0}", logMessage); 
     w.WriteLine("-------------------------------"); 
     // Update the underlying file. 
     w.Flush(); 
    } 
} 

だから、今あなたが持っているスレッドセーフ TextWriter、スレッドセーフ Logger

意味がありますか?

+7

このようにする場合は、 'TextWriter.SyncTextWriter'も削除する必要があります。 'SyncTextWriter'のすべてのメソッド呼び出しはインスタンスをロックします。独自のLoggerクラスのメソッドで 'TextWriter'への呼び出しをラッピングする場合、これは必要ありません。生の 'SyncTextWriter'を周回している場合、次のようにデッドロックすることができます:' lock(textWriter){textWriter.WriteLine( "test"); // oops deadlock' –

+0

良い説明、ありがとう。 –

+2

@jake - agree。すでにロックされている場合は冗長です。 – x0n

5

TextWriter.Synchronizedを呼び出すと、TextWriterの単一のインスタンスを保護しますが1つの「ログ」コールがファイル内に一緒にとどまるように、書き込みを同期させません。

あなたが複数のスレッドから(内部TextWriterインスタンスを使用するかLogWriteを呼び出した場合、個々のWriteLine呼び出しは、あなたの日付と時刻のスタンプが使用できなくなり、織り込ますることができます。

私は個人的にこれのために既に存在する第三者ロギングソリューションを使用します。これがオプションでない場合は、フレームワークのTextWriter.Synchronizedラッパーを使用するよりも、これを自分で(単純なロックでも)同期するほうが便利です。

+0

サードパーティ製のソリューションを使用しても問題ありませんが、安全なロギングが機能します。ロックに関しては、ライター自体をロックするか、無害なオブジェクト、つまりロック(オブジェクト)をロックするかについて明確なリソースを見つけることはできませんでしたか?ロックソリューションを以前は動作させることができませんでした。 –

+0

@Sean:一般的に、私は通常、その目的のために作成された専用オブジェクトをロックすることをお勧めします。これは、他のオブジェクト(すなわち、ライター)が*他のオブジェクトに見える場合、特に重要です。そうであれば、あなたはそれらをロックしたくありません。プライベートオブジェクトを使用すると、後でオブジェクトを心配することなく公開することができます。 –

1

あなたはこのクラス(.NET 2.0の一部)を調べて、独自のロガーを「作成」する必要はありません。あなたは、テキストファイルにログを可能にするイベントビューなど

http://msdn.microsoft.com/en-us/library/system.diagnostics.tracesource.aspx

あなたの「ログイン」方法は、(「TraceSourceの」と呼ばれるintermalメンバ変数があると仮定して)このような何かを見ることができます:

public void Log(TraceEventType eventType, string message) 
    { 
     this.traceSource.TraceEvent(eventType, 0, message); 
     this.traceSource.Flush(); 
    } 

これをサポートするのは、TraceSourceに名前を付け、いくつかの設定を持つ設定セクションです。あなたのロガーにTraceSourceを構築すると、その設定で指定されているトレースソースの1つを使ってインスタンス化していると仮定します。

<system.diagnostics> 
<sources> 
    <source name="Sample" switchValue="Information,ActivityTracing"> 
    <listeners> 
     <add name="file" 
     initializeData="C:\temp\Sample-trace.log" 
     traceOutputOptions="DateTime" 
     type="System.Diagnostics.TextWriterTraceListener, System, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"/> 
    </listeners> 
    </source> 
</sources> 

また、あなたのロガーは静的なことはありません。代わりに、Dependency Injection/IOCのためにEnterprise Library 5.0 Unityを使用してください。

希望すると便利です。あなたのコードをインストルメントする簡単な方法を探しているなら

0

、施設はすでに.NET内に存在する:

http://msdn.microsoft.com/en-us/library/system.diagnostics.trace.aspx

また、サードパーティ製のツールはあなたにログインするための堅牢なソリューションを提供します。例には、log4net,nLog、およびEnterprise Libraryが含まれます。

は、私は本当に、私は他の回答よりも、ここでは完全に異なるアプローチを取ると、あなたが実際より良いスレッド対応コードの書き方を学びたいと仮定するつもりです:)

1

誰かが、今日のいくつかのログの問題を議論している間に、この投稿に私を指摘しました。私たちはすでにここでかなり良い答えを持っていますが、まったく同じことを行うLoggerクラスの単純なバージョンを完全にThreadsafeの方法で表示するために私の答えを追加しています。
ここで注目すべき重要な点は、適切なlock内にファイルを書き込んでいるため、スレッドの安全性のためにはTextWriter.Synchronizedが必要でないことです。

注:これはすでにx0nの回答のコメントセクションで説明しています。

public static class Logger 
{ 
    static readonly object _locker = new object(); 

    public static void Log(string logMessage) 
    { 
     try 
     { 
      var logFilePath = Path.Combine(@"C:\YourLogDirectoryHere", "Log.txt"); 
      //Use this for daily log files : "Log" + DateTime.Now.ToString("yyyy-MM-dd") + ".txt"; 
      WriteToLog(logMessage, logFilePath); 
     } 
     catch (Exception e) 
     { 
      //log log-exception somewhere else if required! 
     } 
    } 

    static void WriteToLog(string logMessage, string logFilePath) 
    { 
     lock (_locker) 
     { 
      File.AppendAllText(logFilePath, 
        string.Format("Logged on: {1} at: {2}{0}Message: {3}{0}--------------------{0}", 
        Environment.NewLine, DateTime.Now.ToLongDateString(), 
        DateTime.Now.ToLongTimeString(), logMessage)); 
     } 
    } 
} 

は単に

Logger.Log("Some important event has occurred!"); 

として呼び出し、何かをログに記録し、それがこの

のようなログエントリがログオンようになります:で2015年10月7日:午前2時11分23秒
メッセージ:重要なイベントが発生しました。
--------------------

関連する問題