Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > comp.lang.java.programmer > #15381
| 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> |
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 | Next — Previous in thread | Next in thread | Find similar | Unroll 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