views:

58

answers:

1

Hi,

I have a simple class called Job with a public ctor and public function called Run().

Run() will do some work including making a request to a 3rd party vendor which cost some $$$. Before it makes the request, it first checks the SQL Server DB to see if the data is already there. After making the request, it puts the data in the DB.

This is an ASP.NET front-end with .NET WCF Windows Service back-end. The request comes in through WCF call, to which I made my contract an InstanceContextMode.Singleton. For those not familiar with WCF - this just means my contract will queue any requests until the current one is finished. In here I simply create an instance of the Job class, send it the input parameters from the user in the ctor, and call the Run() function.

This works great, and since it's a Singleton WCF service, it stops duplicate requests from going to the vendor and costing $$ that doesn't need to be spent. The jobs run fast enough to make it so that only running one job at a time is just fine.

However, this is still a bit smelly to me. The job class is completely independent from the WCF service, and any caller for that matter, and should be re-usable if I wanted.

How can I make the job class, 'self thread safe' so to say? What I mean is - the way it's designed now, the caller of the job class has to make sure he doesn't run two jobs at the same time, for fear of creating an unneccesary dupe request.

Is there a way to do this, with using pure thread locking, inside of the job class itself? So that if a user of my class created 2 instances of Job class and spun off 2 threads calling Run(), it would queue the second guy from proceeding until the first is finished? (or any number of calls for that matter). I can't just Lock() for the whole Run function, since like in the example I just gave they would just spin the Run call into seperate threads making the Lock useless. I think there is something obvious and simple I am missing here... ??

Note - making it 'self managed' across procceses is not what I'm concerned about. I'm concerned about the same process using my Job class in multiple threads. For the former, I would just put the 'check DB / call to vendor / insert DB' into a seperate service.

Thanks

EDIT: thanks Dave for the answer. Here is the code to prove that it works: just remove the SyncLock line to see it work in action. (Sorry, we use VB at work =P)

Public Class ThreadTest
    Private Shared syncObj As New Object()

    Private id As String
    Public Sub New(ByVal id As String)
        Me.id = id
    End Sub

    Public Sub Run()
        'remove this line to see the output change
        SyncLock syncObj
            For i As Integer = 0 To 1000
                Console.Write(id)
            Next
        End SyncLock
    End Sub
End Class

Module Module1
    Sub Main()
        Dim tt As New ThreadTest("1")
        Dim tt2 As New ThreadTest("2")

        Dim thread1 As New Threading.Thread(AddressOf tt.Run)
        Dim thread2 As New Threading.Thread(AddressOf tt2.Run)

        thread1.Start()
        thread2.Start()

        Threading.Thread.Sleep(Threading.Timeout.Infinite)
    End Sub
End Module
+3  A: 

It depends on how your application is structured, but you might be able to get away with locking on a private static object variable in your Job class.

private static readonly object lockObject = new object();

public void Run()
{
    lock (lockObject)
    {
        // Do a bunch of stuff here.
    }
}
Dave
dferraro
Glad it was helpful. Just an FYI - it is generally a bad idea to lock(this) in any case if your instance is public--can lead to deadlocks. See http://msdn.microsoft.com/en-us/library/c5kehkcz(VS.80).aspx
Dave
thanks for the note - i have another question for you - any error checking I need to do here (like in my above example)?? E.G., how does SyncLock/Lock work, in the sense of failures? How do I make sure the lock is released here in this code?? Is it guarenteed or do I need some special cases...????
dferraro
No, under the hood, the compiler translates SyncLock into a Monitor.Enter / Exit call wrapped in try / finally. See the end of the remarks at http://msdn.microsoft.com/en-us/library/de0542zz.aspx
Dave
thanks again Dave! BTW, only temporarily unmarked the answer as I wasn't sure if it would fall into the abyss without it... =P
dferraro