Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > comp.lang.java.programmer > #15381

Re: Infinite loop from HashMap.keySet iteration.

From Robert Klemme <shortcutter@googlemail.com>
Newsgroups comp.lang.java.programmer
Subject Re: Infinite loop from HashMap.keySet iteration.
Date 2012-06-18 18:34 +0200
Message-ID <a493k2FpcfU1@mid.individual.net> (permalink)
References <EHoCr.13285$ji7.6276@newsfe20.iad> <313a279f-0e72-4431-9879-c0deb4385f1c@googlegroups.com> <RnIDr.19181$l_1.6246@newsfe21.iad>

Show all headers | View raw


On 06/18/2012 06:08 PM, Daniel Pitts wrote:
> On 6/18/12 4:42 AM, Robert Klemme wrote:

>> [refactoring from keySet() to entrySet()]

> Agreed. However, the code I was handed used keySet(). I didn't feel like
> refactoring this (for many reasons).

For example?

> Nope, that is not, in fact, the original author's intent. There was no
> other synchronizing on the resultMap. The original author was only
> worried about protecting the key, not the whole map. This of course is
> not sufficient, but it is what they implemented.

The key is a String which does not need synchronization because it is 
immutable.  Did you mean to say "factory" instead?  I have no idea what 
ResultFactory actually does but getName() looks like something which 
always returns the same name.  If that was the case then no 
synchronization would be necessary.  And if getResult() involves a 
calculation which needs to be thread safe then that method could be 
synchronized.  Can you share more details about that class?

>> Result result = resultsMap.get(factory.getName());
>> if (result == null) {
>> result = new Result();
>> resultsMap.put(factory.getName(), result);
>> }
>> result.recordResult(factory.getResult());
>> }
>> }
>>
>> Or the synchronize(resultsMap) was simply forgotten.
> Exactly, which is why I ensure resultsMap is a synchronized Map instead.

This won't make the code entirely thread safe - at least if you want to 
ensure that the fist thread which detects the missing entry also adds it 
to the Map and - maybe more important - that getResult() is invoked only 
once per key.  Collections.synchronizedMap() only ensures all accesses 
are synchronized - not more.

Kind regards

	robert

Back to comp.lang.java.programmer | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-14 09:54 -0700
  Re: Infinite loop from HashMap.keySet iteration. markspace <-@.> - 2012-06-14 10:17 -0700
    Re: Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-14 10:34 -0700
  Re: Infinite loop from HashMap.keySet iteration. Roedy Green <see_website@mindprod.com.invalid> - 2012-06-14 11:34 -0700
    Re: Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-14 12:19 -0700
  Re: Infinite loop from HashMap.keySet iteration. Robert Klemme <shortcutter@googlemail.com> - 2012-06-18 04:42 -0700
    Re: Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-18 09:08 -0700
      Re: Infinite loop from HashMap.keySet iteration. Robert Klemme <shortcutter@googlemail.com> - 2012-06-18 18:34 +0200
        Re: Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-18 13:32 -0700
          Re: Infinite loop from HashMap.keySet iteration. Robert Klemme <shortcutter@googlemail.com> - 2012-06-18 23:26 +0200
            Re: Infinite loop from HashMap.keySet iteration. Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2012-06-18 15:30 -0700

csiph-web