This discussion is archived
1 2 Previous Next 17 Replies Latest reply: Nov 2, 2012 4:05 PM by EJP RSS

Is this method Thread safe?

LosLobo Newbie
Currently Being Moderated
Is this method Thread Safe as a static method in a multi-threaded environment to use as common code?
It's just a simple method to count non-null entries.

public static int countNonNullEntries(HashMap hm){
if(hm==null)return 0;
int count=0;
for(int i=0; i<hm.size(); i++) {
if(hm.get(i)!=null)
{ count++;}
}
return count;
}
  • 1. Re: Is this method Thread safe?
    gimbal2 Guru
    Currently Being Moderated
    The map can still be altered by another thread while this code is running through it. So no, it is not guaranteed thread safe.

    Note: the iteration code is a bit weird. Now it looks like you're using a HashMap as if it is an ArrayList.
  • 2. Re: Is this method Thread safe?
    LosLobo Newbie
    Currently Being Moderated
    I'm just doing this check because there are cases where there is a key with no associated value.
    How do you know this is not thread safe?
    Would you have to create a multithreaded environment with multiple instances and run it say thousands of times until an incorrect response comes back to prove it?
  • 3. Re: Is this method Thread safe?
    gimbal2 Guru
    Currently Being Moderated
    968981 wrote:
    How do you know this is not thread safe?
    Eh? If you repeat the question I'm just going to repeat the answer:
    The map can still be altered by another thread while this code is running through it. So no, it is not guaranteed thread safe.
    But I said not guaranteed. I can't make any claims based on a very non-specific question devoid of context other than that it is a "multithreaded environment".
  • 4. Re: Is this method Thread safe?
    LosLobo Newbie
    Currently Being Moderated
    I'm asking, how would prove what you are saying is this case?
  • 5. Re: Is this method Thread safe?
    gimbal2 Guru
    Currently Being Moderated
    Questions consisting of one short sentence are hardly ever complete and by extension - clear.

    Let me rephrase: huh?
  • 6. Re: Is this method Thread safe?
    ++sja Explorer
    Currently Being Moderated
    The problem is not that you are not understanding the answer you are getting.

    The problem is that you do not understand the question you are asking.

    It's all very zen.
  • 7. Re: Is this method Thread safe?
    LosLobo Newbie
    Currently Being Moderated
    Multi-threaded like say running on Amazon, with one million users hitting the same method at the same time. : )
  • 8. Re: Is this method Thread safe?
    DrClap Expert
    Currently Being Moderated
    Let me explain to you the question you asked.

    "Not thread-safe" means there could be situations in which race conditions or other defects in synchronization could cause unexpected results. Now, I have phrased that rather imprecisely, and experts in multiprocessing would certainly phrase it better. However notice the word "could" in there.

    It's not necessary to post an SSCCE which reliably demonstrates such unexpected results every time it runs. Such a thing would usually be impossible. It's only necessary to point at a situation which could (there's that word again) arise in a real situation. So hollering about "proof" means you haven't understood this.

    And one of those possible unsafe situations has indeed been pointed out. That doesn't mean that the situation will occur with predictable frequency in any environment. It may not occur at all in some environments. The point is, there are environments in which it is possible the situation will occur. That is all that is necessary to qualify the code as "not thread-safe".
  • 9. Re: Is this method Thread safe?
    LosLobo Newbie
    Currently Being Moderated
    So, is it fair to say this...
    If results from this method must return absolutely precise values, a best practice would be to either place a synchronized keyword on the method or refractor the method into an instance method, instead of a static method.
    Is this a fair conclusion?

    Is this also fair? If fault tolerance permits this method to choke one a large number say 100k times then its ok to leave the method as is?
  • 10. Re: Is this method Thread safe?
    LosLobo Newbie
    Currently Being Moderated
    P.S. I wasn't asking you to personally prove it. I was asking theoretically what one would do if they wanted to test this scenario. : )
  • 11. Re: Is this method Thread safe?
    DrClap Expert
    Currently Being Moderated
    968981 wrote:
    P.S. I wasn't asking you to personally prove it. I was asking theoretically what one would do if they wanted to test this scenario. : )
    In practice it isn't possible to test it, for exactly the same reason it isn't possible to prove it. Often the way you will find out that you have thread-safety problems is that strange things start to happen on your server when it is heavily loaded.
  • 12. Re: Is this method Thread safe?
    EJP Guru
    Currently Being Moderated
    It is thread safety that has to be proven, not the lack of it. The onus of proof and testing are both on you, not this forum. If you think it is thread-safe, you prove it and test it.
  • 13. Re: Is this method Thread safe?
    DrClap Expert
    Currently Being Moderated
    968981 wrote:
    If results from this method must return absolutely precise values, a best practice would be to either place a synchronized keyword on the method or refractor the method into an instance method, instead of a static method.
    Is this a fair conclusion?
    No, it isn't. For one thing "return absolutely precise values" is a phrase which doesn't really mean anything in a multi-processing world. For example, in a multi-processing world you can't even tell which of two events happened first unless you do some careful processing to make that clear. When you read the API docs for the java.util.concurrent package you'll see the phrase "happen-before" mentioned frequently, with a link to a densely-written chunk of text which refers to a chapter of the Java language spec.

    So no, just tossing in the synchronized keyword isn't necessarily the best thing to do. And deciding whether the method should have been static or not has nothing to do with synchronization and everything to do with object-oriented design.

    If you're going to use a HashMap for a Map which is going to be updated by more than one thread simultaneously, the very least you could do would be to use a ConcurrentHashMap. But that's just a start. You have to decide exactly what you want from that code you posted. For example does it matter that after you determined that 4 is a key, but before you finish looking at all of the other keys some other thread removes that entry?
  • 14. Re: Is this method Thread safe?
    jtahlborn Expert
    Currently Being Moderated
    if the HashMap passed in is not shared between threads, this method is thread-safe. if it is shared (and mutable), then the method is not thread-safe. and by "not thread-safe", i don't mean it may just give incorrect values, i mean it may throw random, "impossible" exceptions due to internal inconsistency in the HashMap.
1 2 Previous Next

Legend

  • Correct Answers - 10 points
  • Helpful Answers - 5 points