0

A follow-up to this post. My goal is to have only one Calculate at a time, so I have added a SyncLock:

Public Sub Calculate(Optional inBack As Boolean = True)
    If Not inBack Then
        InternalCalculate(-1, False)
    Else
        If CalcThread IsNot Nothing Then
            CalcThread.Abort() ' yes, I will replace this
            CalcThread = Nothing ' XXX
        End If
        If CalcThread Is Nothing Then
            CalcThread = New Threading.Thread(AddressOf InternalCalculate)
            CalcThread.IsBackground = True
        End If
        CalcThread.Start()
    End If
End Sub

Private Sub InternalCalculate(Optional Line As Integer = -1, Optional isBack As Boolean = True)
    Dim Lock As New Object
    SyncLock Lock
         Threading.Thread.MemoryBarrier() ' do this BEFORE a write, right?
         isRunning = true
        'do the expensive stuff
    End SyncLock
End Sub

Note the isBack. If this is false the code should just run in main. This is commonly used when recalculating a single Line. So my question is about the safety of these two lines:

SyncLock Lock
Threading.Thread.MemoryBarrier()

It is not clear to me in the documentation what happens if I call these in code running in main. I have added the code, and it seems to run OK, but I want to make sure I'm not opening myself to another gotcha, like Abort. Are these OK for both threaded and non-threaded uses?

Community
  • 1
  • 1
Maury Markowitz
  • 9,082
  • 11
  • 46
  • 98
  • The term `thread` is short for `thread of execution`, and in a nutshell, everything is a thread, hence the term "single threaded" for a program that doesn't use threads. `main` itself runs in a thread, it may simply be the *only* thread and doesn't any special work. Every process has a default/initial thread. – kfsone Sep 21 '16 at 21:20
  • Sure, but a *Sync*Lock is for *synchronizing* threads. If there's only one thread there's nothing to *synchronize*. Thus my question. – Maury Markowitz Sep 22 '16 at 13:00

1 Answers1

1

Yes it's safe to use them in single threaded code.

There are issues however with your InternalCalculate which will allow it to execute code concurrently. Every call to InternalCalculate creates a new Lock object. Synclock will only block threads if these use the same object so make Lock a readonly member variable.

If you then execute multiple threads then the second, third, fourth etc will wait until the first one to obtain the lock exits the sync lock block. Then the others will go. So if this is code that should only be executed once you should check out the double check lock pattern.

Instead of adding MemoryBarriers for the reading / writing of volatile data, I'd suggest you use System.Threading.Thread.VolatileRead and System.Threading.Thread.VolatileWrite. Then you don't need to remember which order to do your reads / writes in.

FloatingKiwi
  • 4,408
  • 1
  • 17
  • 41
  • Thank you @FloatingKiwi! I will make the change to Lock ASAP - that code comes directly from an MS example and I was curious about that issue myself. Is it enough to simply move the declaration outside the Sub and into the enclosing object? Do I really need a property? – Maury Markowitz Sep 22 '16 at 12:02
  • A follow-up: since the consumers of the isCalculating bool are spread throughout the app, does that mean they would all need to use VolatileRead? Or is a VolitileWrite inside the thread enough to ensure the consumers in main are OK? – Maury Markowitz Sep 22 '16 at 12:57
  • My suggestion was a readonly field, not a readonly property. This just makes it clear that you're creating 1 lock object per instance of your class. Regarding the volatile code, I'd restructure the code so as not to need it at all. For instance change calculate to a function that returns your result. Without seeing what you're calculating / how youre interacting with your objects its difficult to tell. – FloatingKiwi Sep 22 '16 at 13:05