Note that there are some explanatory texts on larger screens.

plurals
  1. POShould I double check before and after locking a list?
    text
    copied!<p>I have an in-application service which allows me to feed it messages from various sources, which will be put into a simple list. The service, running in its own thread, will, periodically, process all messages in the list into various files; one file for each source, which are then managed for size.</p> <p>My question is about the proper way to check for messages and performing a lock around the code which accesses the list. There are only two places which access the list; one is where a message is added to the list and the other is where the messages are dumped from the list into a processing list.</p> <p>Adding a message to the list:</p> <pre><code>Public Sub WriteMessage(ByVal messageProvider As IEventLogMessageProvider, ByVal logLevel As EventLogLevel, ByVal message As String) SyncLock _SyncLockObject _LogMessages.Add(New EventLogMessage(messageProvider, logLevel, Now, message)) End SyncLock End Sub </code></pre> <p>Processing the list:</p> <pre><code>Dim localList As New List(Of EventLogMessage) SyncLock _SyncLockObject If (_LogMessages.Count &gt; 0) Then localList.AddRange(_LogMessages) _LogMessages.Clear() End If End SyncLock ' process list into files... </code></pre> <p>My questions are: should I do a double check when I am processing the list, see below? And why? Or why not? And are there any dangers in accessing the list’s count property outside of the lock? Are either of the methods better or more efficient? And why? Or why not?</p> <pre><code>Dim localList As New List(Of EventLogMessage) If (_LogMessages.Count &gt; 0) Then SyncLock _SyncLockObject If (_LogMessages.Count &gt; 0) Then localList.AddRange(_LogMessages) _LogMessages.Clear() End If End SyncLock End If ' process list into files... </code></pre> <p>I understand that in this particular case, it may not matter if I do a double check given the fact that, outside of the processing function, the list can only grow. But this is my working example and I’m trying to learn about the finer details of threading. </p> <p>Thank you in advance for any insights…</p> <hr> <p>After some further research, thank you 'the coon', and some experimental programming, I have some further thoughts.</p> <p>Concerning the <a href="http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx" rel="nofollow" title="ReaderWriterLockSlim">ReaderWriterLockSlim</a>, I have the following example which seems to work fine. It allows me to read the number of messages in the list without interfering with other code which may be trying to read the number of messages in the list, or the messages themselves. And when I desire to process the list, I can upgrade my lock to write mode, dump the messages into a processing list and process them outside of any read/write locks, thus not blocking any other threads which may want to add, or read, more messages.</p> <p>Please note, that this example uses a simpler construct for the message, a String, as opposed to the previous example which used a Type along with some other metadata. </p> <pre><code>Private _ReadWriteLock As New Threading.ReaderWriterLockSlim() Private Sub Process() ' create local processing list Dim processList As New List(Of String) Try ' enter read lock mode _ReadWriteLock.EnterUpgradeableReadLock() ' if there are any messages in the 'global' list ' then dump them into the local processing list If (_Messages.Count &gt; 0) Then Try ' upgrade to a write lock to prevent others from writing to ' the 'global' list while this reads and clears the 'global' list _ReadWriteLock.EnterWriteLock() processList.AddRange(_Messages) _Messages.Clear() Finally ' alway release the write lock _ReadWriteLock.ExitWriteLock() End Try End If Finally ' always release the read lock _ReadWriteLock.ExitUpgradeableReadLock() End Try ' if any messages were dumped into the local processing list, process them If (processList.Count &gt; 0) Then ProcessMessages(processList) End If End Sub Private Sub AddMessage(ByVal message As String) Try ' enter write lock mode _ReadWriteLock.EnterWriteLock() _Messages.Add(message) Finally ' always release the write lock _ReadWriteLock.ExitWriteLock() End Try End Sub </code></pre> <p>The only problem I see with this technique is that the developer must be diligent about acquiring and releasing the locks. Otherwise, deadlocks will occur.</p> <p>As to whether this is more efficient than using a <a href="http://msdn.microsoft.com/en-us/library/3a86s51t%28v=vs.100%29.aspx" rel="nofollow" title="SyncLock">SyncLock</a>, I really could not say. For this particular example and its usage, I believe either would suffice. I would not do the double check for the very reasons ‘the coon’ gave about reading the count while someone else is changing it. Given this example, the <a href="http://msdn.microsoft.com/en-us/library/3a86s51t%28v=vs.100%29.aspx" rel="nofollow" title="SyncLock">SyncLock</a> would provide the same functionality. However, in a slightly more complex system, one where multiple sources might read and write to the list, the <a href="http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx" rel="nofollow" title="ReaderWriterLockSlim">ReaderWriterLockSlim</a> would be ideal.</p> <hr> <p>Concerning the <a href="http://msdn.microsoft.com/en-us/library/dd267312.aspx" rel="nofollow">BlockingCollection</a> list, the following example works like the one above.</p> <pre><code>Private _Messages As New System.Collections.Concurrent.BlockingCollection(Of String) Private Sub Process() ' process each message in the list For Each item In _Messages ProcessMessage(_Messages.Take()) Next End Sub Private Sub AddMessage(ByVal message As String) ' add a message to the 'global' list _Messages.Add(message) End Sub </code></pre> <p>Simplicity itself…</p>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload