Solved

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

Posted on 2013-11-22
11
534 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
Comment Utility
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
Comment Utility
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
Comment Utility
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
Comment Utility
...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
Comment Utility
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
Enabling OSINT in Activity Based Intelligence

Activity based intelligence (ABI) requires access to all available sources of data. Recorded Future allows analysts to observe structured data on the open, deep, and dark web.

 

Author Comment

by:XTO
Comment Utility
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
Comment Utility
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
Comment Utility
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
Comment Utility
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
Comment Utility
Would putting a lock in the second method make it thread safe?
0
 
LVL 85

Expert Comment

by:Mike Tomlinson
Comment Utility
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

What Should I Do With This Threat Intelligence?

Are you wondering if you actually need threat intelligence? The answer is yes. We explain the basics for creating useful threat intelligence.

Join & Write a Comment

If you’re thinking to yourself “That description sounds a lot like two people doing the work that one could accomplish,” you’re not alone.
Real-time is more about the business, not the technology. In day-to-day life, to make real-time decisions like buying or investing, business needs the latest information(e.g. Gold Rate/Stock Rate). Unlike traditional days, you need not wait for a fe…
Viewers will learn how to properly install Eclipse with the necessary JDK, and will take a look at an introductory Java program. Download Eclipse installation zip file: Extract files from zip file: Download and install JDK 8: Open Eclipse and …
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 …

744 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

14 Experts available now in Live!

Get 1:1 Help Now