Solved

Multithreading lock object, Two static public methods can call a static private method

Posted on 2013-11-22
11
548 Views
Last Modified: 2013-11-22
Both of the static public methods below can call the static private method.
I wasn't sure if I should put the lock on all the static methods, or just the private static method.
I can see a case why I should lock the resources in the public static methods also.
For example, what if the query object in one of the public static methods gets used by a second client call before the first client call is finished? Might that not corrupt the return argument to the second client?

Here is the code:

public static class QueryForFinder
{ 
    public static Query GetParameterQuery(
        string querySearch,
        bool returnGeometry = true)   
    {
        Query query;
        query = SetParameterQuery(querySearch, returnGeometry);
        query.OutFields.Add("*");

        return query;             
    }

    public static Query GetCollectionParameterQuery(
        string[] queryCollection, 
        string querySearch, 
        bool returnGeometry = true)
    {
        Query query;
        query = SetParameterQuery(querySearch, returnGeometry);
        query.OutFields.AddRange(queryCollection);

        return query;             
    }

    private static Query SetParameterQuery(
        string querySearch, 
        bool returnGeometry)
    {
        object SpinLock = new object();

        lock (SpinLock)
        {
            Query query = new Query();
            query.Where = querySearch;
            query.ReturnGeometry = returnGeometry;

            return query;
        }
    }
}
}

Open in new window


Should the public methods above also lock their Query object resources?
Are there any other ways to improve the code above?
0
Comment
Question by:XTO
  • 5
  • 5
11 Comments
 

Author Comment

by:XTO
ID: 39669427
I just thought of this.
I sense that the class above wants to become refactored into a factory pattern that returns Query objects.
Does anyone else get that, or am I misunderstanding the factory pattern?
Apologies. This was intended on being a threading question, not a design pattern question; but I just noticed that.
0
 
LVL 15

Assisted Solution

by:unknown_routine
unknown_routine earned 200 total points
ID: 39669442
No they don't have to. If your public methods were spanning new threads you may needed more locks but this is not the case for you.|

If you lock a  public method , that method will act as synchronous for the time of the lock and any other methods that it calls will be also synchronous.

Your code looks good to me . Do you see any actual issue/bug?
0
 

Author Comment

by:XTO
ID: 39669639
Thanks,
re: "If you lock a  public method , that method will act as synchronous for the time of the lock and any other methods that it calls will be also synchronous."

Yes, I would like those methods to be called synchronously because I'm worried that one of the methods might get called by the client a second time before the first one has finished. I fear then that the second client call might get the Query object that was created for the first client call.

Here is some pseudocode.

public static XType PublicMethod(XArg arg)
{
   XType publicResource;
   // Some stuff happens here.
   publicResource = ManipulateResource(arg);
   // Do stuff to publicResource. 
   return publicResource;   
}

private static XType PrivateMethod(XArg arg)
{
   lock(SpinLock)
   {
       XType privateResource = new XType();
       // Do stuff to privateResource with arg      
      return privateResource;
   }
}

Open in new window


Imagine that client1 first calls PublicMethod(), and next client2 then calls PublicMethod().
Suppose then that inside of PublicMethod(), client2 gets to call the ManipulateResources method prior to client1.
Then, while the client2 call is in the do stuff part of the code, and before privateResource gets returned for client2, the client1 call accesses the ManipulateResources method and gets a response which alters privateResource.
Then when the client2 call returns privateResource, it returns the object intended for the client1 call.
I don't know if that is something that could realistically happen, but that's what I'm worried about.
0
 
LVL 85

Expert Comment

by:Mike Tomlinson
ID: 39669640
...but you haven't actually locked anything in your code.  Your "SpinLock " variable is declared locally, which means each thread will get its own copy.  Therefore each thread will run without waiting for anybody else, since nobody else has access to that local variable.

"lock" only works if multiple threads try to access the same object.  To make it work you'd have to move "SpinLock" out to class level.
0
 
LVL 85

Expert Comment

by:Mike Tomlinson
ID: 39669665
Each thread will have it's own copies of all the local variables so the problem you described doesn't exist.

The only time this is an issue is if the things being manipulated are static in nature themselves.  So if the methods are changing the same static members, then yes, it's possible to get an uncertain state.  If the data being manipulated isn't getting shared somehow between threads or calls, then you don't need to worry about this.
0
DevOps Toolchain Recommendations

Read this Gartner Research Note and discover how your IT organization can automate and optimize DevOps processes using a toolchain architecture.

 

Author Comment

by:XTO
ID: 39669687
re: "... this is an issue is if the things being manipulated are static...."

So, this pseudocode below could be a problem:

private static XType publicResource;

public static XType PublicMethod(XArg arg)
{  
   // Some stuff happens here.
   publicResource = ManipulateResource(arg);
   // Do stuff to publicResource. 
   return publicResource;   
}

private static XType PrivateMethod(XArg arg)
{
   lock(SpinLock)
   {
       XType privateResource = new XType();
       // Do stuff to privateResource with arg  
      return privateResource;
   }
}

Open in new window

0
 
LVL 85

Expert Comment

by:Mike Tomlinson
ID: 39669794
One is method is manipulating a static member "publicResource".

The other is manipulating a local variable called "privateResource".  The lock in PrivateMethod isn't doing anything unless XType itself has static members inside it that need thread synchronization.

Without further knowledge, the two methods appear to be isolated with respect to each other, and there is no problem.

Now, if multiple threads were calling the first method, PublicMethod(), then you could potentially have an issue as each thread could change the state of "publicResource" without knowledge of what the others are doing to it, putting it into an invalid state.

So PublicMethod() is really the one that should have lock in it, to control access to the static member "publicResource".

Hope that explanation helps.
0
 

Author Comment

by:XTO
ID: 39669809
Ah, I am so sorry. I messed up the pseudocode.
Here are the two code pieces again below.
I changed the name of PrivateMethod to ManipulateResource so that it gets called from PublicMethod.

What I think is thread safe code:
public static XType PublicMethod(XArg arg)
{
   XType publicResource;
   // Some stuff happens here.
   publicResource = ManipulateResource(arg);
   // Do stuff to publicResource. 
   return publicResource;   
}

private static XType ManipulateResource(XArg arg)
{
   lock(SpinLock)
   {
       XType privateResource = new XType();
       // Do stuff to privateResource with arg  
      return privateResource;
   }
}

Open in new window



And what I think is not thread-safe code:
private static XType publicResource;

public static XType PublicMethod(XArg arg)
{  
   // Some stuff happens here.
   publicResource = ManipulateResource(arg);
   // Do stuff to publicResource. 
   return publicResource;   
}

private static XType ManipulateResource(XArg arg)
{
   lock(SpinLock)
   {
       XType privateResource = new XType();
       // Do stuff to privateResource with arg  
      return privateResource;
   }
}

Open in new window


I had not put the pseudocode through a compiler.
I think I got it right this time to illustrate the point behind my question.

So, the first code is thread-safe and the second is not, correct?
0
 
LVL 85

Accepted Solution

by:
Mike Tomlinson earned 250 total points
ID: 39669858
The first is thread safe, but the lock isn't necessary.  It's safe because all the variables are locally declared and each thread will create their own instances to play with.

The second is not thread safe as multiple threads could be running the "// Do stuff to publicResource" part simultaneously and making changes to the static publicResources member that don't make sense.
0
 

Author Comment

by:XTO
ID: 39669867
Would putting a lock in the second method make it thread safe?
0
 
LVL 85

Expert Comment

by:Mike Tomlinson
ID: 39669921
Yes...but you'd have to make sure that no other methods are accessing "publicResource" directly, otherwise the lock becomes kinda pointless.
0

Featured Post

DevOps Toolchain Recommendations

Read this Gartner Research Note and discover how your IT organization can automate and optimize DevOps processes using a toolchain architecture.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

A short article about problems I had with the new location API and permissions in Marshmallow
Performance in games development is paramount: every microsecond counts to be able to do everything in less than 33ms (aiming at 16ms). C# foreach statement is one of the worst performance killers, and here I explain why.
In this fifth video of the Xpdf series, we discuss and demonstrate the PDFdetach utility, which is able to list and, more importantly, extract attachments that are embedded in PDF files. It does this via a command line interface, making it suitable …
In this seventh video of the Xpdf series, we discuss and demonstrate the PDFfonts utility, which lists all the fonts used in a PDF file. It does this via a command line interface, making it suitable for use in programs, scripts, batch files — any pl…

861 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

22 Experts available now in Live!

Get 1:1 Help Now