Is it a good idea to put frequently used file I / O operations in the SyncLock block?

Say I have code that does this:

Public Function AppendToLogFile(ByVal s As String) As Boolean Dim success As Boolean = True Dim fs As IO.FileStream = Nothing Dim sw As IO.StreamWriter = Nothing Static LogFileLock As New Object() SyncLock LogFileLock Try fs = New IO.FileStream(LogFilePath) sw = New IO.StreamWriter(fs) sw.WriteLine(s) Catch ex As Exception success = False Finally If Not sw Is Nothing Then sw.Close() If Not fs Is Nothing Then fs.Close() End Try End SyncLock Return success End Function 

First of all: the problem is that I have what Try / Catch / finally blocks inside SyncLock?

Secondly: suppose this code is triggered in an event, potentially many times over a short timeframe, say ten times in one second. Is it good SyncLock, or does it make sense to add a line to the queue, and then write all the lines from the queue to a timer file that turns off, say, every second?

+4
source share
2 answers

This looks good to me at first glance, with two caveats:

  • Static elements use a kind of thread-safe locking already behind the scenes. Thus, instead of explicitly locking, you can simply contact the existing lock. I'm not quite sure what it would be like.
  • Do not return status codes. Let the exception extend to the appropriate level. Once you do this, you can rewrite your code as follows:

.

 Public Sub AppendToLogFile(ByVal s As String) As Boolean Static LogFileLock As New Object() SyncLock LogFileLock Using sw As New IO.StreamWriter(LogFilePath) sw.WriteLine(s) End Using End SyncLock End Sub 

These are all functions in less than half of the code. The only difference is that you should handle the exception in the calling code, and not check the return status.

+4
source

In your case, locking is fine as long as the log file is written relatively infrequently. In other words, if every successful operation is logged, it may not be a very good design. If only failed operations are logged, this can be more than enough.

Also, if you write this journal often, you probably want to refer to "StreamWriter" in a shared variable.

 Public NotInheritable Class Log Private Shared m_LogLock As New Object Private Shared m_Log As StreamWriter Public Shared Sub WriteLog(ByVal message As String) SyncLock m_LogLock If m_Log Is Nothing Then m_Log = New StreamWriter("pathAndFileName") End If m_Log.WriteLine(message) m_Log.Flush() End SyncLock End Sub End Class 
+4
source

All Articles