Threadsafe Logfile Klasse

DummyScript · Beiträge: 24

Hi!

Ich habe eine Klasse geschrieben, die es ermöglichen soll threadsafe in ein Logfile schreiben zu können. Nur bin ich mir nicht sicher, ob ich das alles richtig gemacht habe. Deswegen würde ich euch bitten, ein kleines "Review" über meine Klasse zu halten.

Kurze Erklärung: Ich benutze Queue um eine Warteschlange zu realisieren. Per Queue.Synchronize() erstelle ich einen Wrapper um die Warteschlange der sie threadsafe machen sollte. Jedesmal, wenn etwas geloggt werden soll, wird der Eintrag direkt ans Ende der Warteschlange geschrieben. Mit einem Timer der alle 100ms aufgerufen wird, laufe ich die gesamte Warteschlange durch und schreibe den gesamten Inhalt ins Logfile. Danach lösche ich den gesamten Inhalt der Warteschlange.

Ok hier nun der Code:

markieren

C#-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:

public class LogFile
{
    private string m_Filename = null; //Stores the filename and Path of the log file
    private Queue m_LogQueue = new Queue(); //The queue that is used to buffer the log file entries
    private Queue m_LogSyncQueue = null; //The synchronized queue that is thread safe
    private System.Timers.Timer m_LogFileWriterTimer = null; //The class uses a Timer instead of a thread for writing the buffer to the file
    private const int LogFileWriterTimerIntervall = 100; //writes all 100 milliseconds to the file

    public void LogFile(string Filename)
    {
        m_Filename = Filename; //Stores the filename of the logfile

        //Creates or clears an existing log file
        StreamWriter LogFile = new StreamWriter(m_Filename);
        LogFile.Close();

        //Creates the wrapper around m_LogQueue for making it thread safe
        m_LogSyncQueue = Queue.Synchronized(m_LogQueue);

        //Initializes the timer for the LogFileWriter
        m_LogFileWriterTimer = new System.Timers.Timer();
        m_LogFileWriterTimer.Elapsed += new ElapsedEventHandler(LogFileWriterTimerEvent);
        m_LogFileWriterTimer.Interval = LogFileWriterTimerIntervall;
        m_LogFileWriterTimer.Enabled = true;
    }
    
    ~LogFile()
    {
        m_LogFileWriterTimer.Enabled = false;
    }

    public void Log(string msg)
    {
        //Appends an entry to the logging buffer
        m_LogSyncQueue.Enqueue(string.Format("{0}:{1}:{2}:{3}   {4}\r\n", System.DateTime.Now.Hour, System.DateTime.Now.Minute, System.DateTime.Now.Second, System.DateTime.Now.Millisecond, msg);
    }

    private void LogFileWriterTimerEvent(object sender, System.Timers.ElapsedEventArgs e)
    {
        StreamWriter LogFile = File.AppendText(m_Filename);

        //Appends the whole content of the buffer queue
        foreach(Object obj in m_LogSyncQueue)
        {
            LogFile.WriteLine(obj);
        }
        //And deletes the whole buffer
        m_LogSyncQueue.Clear();

        LogFile.Close(); //Everything is written into the file now
    }
}

Ich könnte mir vorstellen, dass es ein Problem gibt wenn genau zwischen dem Ende der foreach-Schleife und m_LogSyncQueue.Clear() aus dem Unterprogramm rausgesprungen wird (wegen eines anderen Threads) und dieser Thread nun Log() aufruft. Dann würde die Nachricht verloren gehen. Die Frage ist, ob dies überhaupt möglich ist?

Noch eine Frage nebenbei, gibt es eigentlich zwischen Threads und Timerevents einen unterschied in der Behandlung? Sprich: Müssen Methoden die in Timerevents aufgerufen werden auch threadsafe sein?

PS: Der Code wurde noch nicht getestet, ich wollte, dass er zuerst in der Theorie funktioniert.

Danke für eure Hilfe!

Moderiert von Christian S.: Code- durch C#-Tags ersetzt

danielf · Verfasst: Mi 04.08.10 15:21

Hallo,

nein das funktioniert so nicht. Du hast zwar lock Mechanismen vorgesehen, verwendest sie aber nirgends.

Außerdem kannst du die LogFile-Klasse ja nur einmal instanzieren, d.h. deiner Anwendung muss überall die Log-Instanz übergeben werden. Ich denke es wäre besser, wenn du ein Singlention-Pattern machst.

Warum du alle 100ms die Logs "nur" raus schreibst verstehe ich auch nicht. Ich denke eine Consumer-Producer-Pattern würde ausreichen. Eine Synchronisation hinzubekommen wird allerdings schwierig. Ich denke theoretisch ist es unmöglich einen globalen Logger zu haben der 100% in der richtigen Reihenfolge logged. Es ist nur möglich so wie die "public void Log(String logEntry)" aufgerufen wird zu loggen. Denn noch kann dieser Aufruf schon verschoben mit einem vorherigen Logaufruf kommen.

Am einfachsten ist, du verwendest Log4Net.

Gruß

Th69 · Verfasst: Mi 04.08.10 15:27

DummyScript, von wirklich "threadsafe" sehe ich in deinem Code aber nicht so viel (außer daß du eine synchronisierte Queue benutzt)!

Bei mehreren Threads kannst du nie wissen, wann der Thread wechselt (nur durch Synchronisations-Mechanismen, z.B. das "lock"-Schlüsselwort - welches intern die Klasse "Monitor" benutzt - kann man programmtechnisch darauf reagieren).

Auch das Timer-Event kann also jederzeit von einem anderen Thread unterbrochen werden!
Du müßtest deshalb explizit mit Queue.Dequeue arbeiten, anstatt mit Queue.Clear die Liste radikal zu löschen (da du ja erkannt hast, daß in der Zwischenzeit ein anderer Thread wieder einen neuen Eintrag über Queue.Enqueue erzeugen kann -)

Du könntest auch evtl. diese generische Sync-Queue benutzen: www.mycsharp.de/wbb2...d.php?threadid=80713

DummyScript · Beiträge: 24

Ich wollte anfangs auch mit Queue.Dequeue() arbeiten, aber dann habe ich gelesen, dass man in einer foreach Schleife die Inhlate bzw. Aufbau des Objekts nicht ändern darf/sollte.

markieren

C#-Quelltext

1:
2:
3:
4:
5:

foreach(Object obj in m_LogSyncQueue)
{
     LogFile.WriteLine(obj);
     m_LogSyncQueue.Dequeue();
}

Demnach wäre obiger Code nicht erlaubt, oder?

Zitat:

Außerdem kannst du die LogFile-Klasse ja nur einmal instanzieren, d.h. deiner Anwendung muss überall die Log-Instanz übergeben werden.

Ja das habe ich beabsichtigt so gemacht.

Zitat:

Warum du alle 100ms die Logs "nur" raus schreibst verstehe ich auch nicht. Ich denke eine Consumer-Producer-Pattern würde ausreichen. Eine Synchronisation hinzubekommen wird allerdings schwierig. Ich denke theoretisch ist es unmöglich einen globalen Logger zu haben der 100% in der richtigen Reihenfolge logged. Es ist nur möglich so wie die "public void Log(String logEntry)" aufgerufen wird zu loggen. Denn noch kann dieser Aufruf schon verschoben mit einem vorherigen Logaufruf kommen.

Aus den von dir genannten Gründen finde ich meine Methode perfekt. Reihenfolge sollte eigentlich bei meiner Methode gegeben sein. Aber im Prinzip nicht soooo wichtig da ich sowieso bei jedem Log-Eintrag einen Timestamp ausgebe.

Moderiert von Christian S.: Code- durch C#-Tags ersetzt

DummyScript · Beiträge: 24

Ok also ich habe die Schleife nun so gelöst:

markieren

C#-Quelltext

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:
32:
33:
34:
35:
36:
37:
38:
39:
40:
41:
42:
43:
44:
45:
46:
47:
48:
49:
50:
51:
52:
53:
54:
55:
56:
57:
58:
59:
60:
61:
62:
63:
64:
65:
66:
67:
68:
69:
70:
71:
72:
73:
74:
75:
76:
77:
78:

public class Logger
{
    private string m_Filename = null; //Stores the filename and Path of the log file
    private Queue m_LogQueue = new Queue(); //The queue that is used to buffer the log file entries
    private Queue m_LogSyncQueue = null; //The synchronized queue that is thread safe
    private System.Timers.Timer m_LogFileWriterTimer = null; //The class uses a Timer instead of a thread for writing the buffer to the file
    private const int LogFileWriterTimerIntervall = 100; //writes all 100 milliseconds to the file

    public Logger(string Filename)
    {
        m_Filename = Filename; //Stores the filename of the logfile

        //Creates the wrapper around m_LogQueue for making it thread safe
        m_LogSyncQueue = Queue.Synchronized(m_LogQueue);

        //Initializes the timer for the LogFileWriter
        m_LogFileWriterTimer = new System.Timers.Timer();
        m_LogFileWriterTimer.Elapsed += new ElapsedEventHandler(LogFileWriterTimerEvent);
        m_LogFileWriterTimer.Interval = LogFileWriterTimerIntervall;
        m_LogFileWriterTimer.Enabled = true;
    }

    ~Logger()
    {
        m_LogFileWriterTimer.Enabled = false;
    }

    public void Log(string msg)
    {
        //Appends an entry to the logging buffer
        m_LogSyncQueue.Enqueue(string.Format("{0}:{1}:{2}:{3}   {4}\r\n", System.DateTime.Now.Hour, System.DateTime.Now.Minute, System.DateTime.Now.Second, System.DateTime.Now.Millisecond, msg));
    }

    private void LogFileWriterTimerEvent(object sender, System.Timers.ElapsedEventArgs e)
    {
        StreamWriter LogFile;
        m_LogFileWriterTimer.Enabled = true;

        if (!File.Exists(m_Filename))
        {
            try
            {
                LogFile = new StreamWriter(m_Filename);
            }
            catch
            {
                return;
            }
        }
        else
        {
            try
            {
                LogFile = File.AppendText(m_Filename);
            }
            catch
            {
                return;
            }
        }

        //Appends the whole content of the buffer queue
        while (true)
        {
            string msg;
            try
            {
                msg = (string)m_LogSyncQueue.Dequeue();
            }
            catch
            {
                LogFile.Close(); //Everything is written into the file now
                return;
            }
            LogFile.WriteLine(msg);
        }
    }
}

EDIT: Mich würde interessieren ob der Code zumindest theoretisch so funktionieren kann? Praktisch funktioniert er nur teilweise. Manche Log-Einträge werden nicht in die Datei geschrieben (hauptächlich die, die gleich nach der Initialisierung der Instanz von LogFile geschloggt werden).

EDIT2: Nun funktioniert es. Hab den Code oben geupdatet. Es steht jedem frei den Code für seine eigenen Zwecke zu Mis-/Gebrauchen

Danke nochmale für eure Hilfe!

Moderiert von Kha: Code- durch C#-Tags ersetzt

Kha · Verfasst: Mi 04.08.10 18:53

Ab 4.0 würde man wohl eher zur ConcurrentQueue<T> greifen, aber ansonsten dürfte deine Lösung nun wirklich thread-safe sein

.

_________________
>λ=

	Mitgliederliste
	Gruppen
	Das Team
	Richtlinien
	Synonyme