Path: csiph.com!usenet.pasdenom.info!gegeweb.org!eternal-september.org!feeder.eternal-september.org!mx04.eternal-september.org!.POSTED!not-for-mail
From: markspace <-@.>
Newsgroups: comp.lang.java.programmer
Subject: Re: Infinite loop from HashMap.keySet iteration.
Date: Thu, 14 Jun 2012 10:17:23 -0700
Organization: A noiseless patient Spider
Lines: 36
Message-ID: <jrd6b5$mh1$1@dont-email.me>
References: <EHoCr.13285$ji7.6276@newsfe20.iad>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Injection-Date: Thu, 14 Jun 2012 17:17:25 +0000 (UTC)
Injection-Info: mx04.eternal-september.org; posting-host="IZQsHU8CwMUPnWgvh4wwWA"; logging-data="23073"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18IcTupm+I0rbjNoqijymZhCsk6m0nfmSQ="
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20120428 Thunderbird/12.0.1
In-Reply-To: <EHoCr.13285$ji7.6276@newsfe20.iad>
Cancel-Lock: sha1:Hq2QTBpdZ4QgtaLVZiYRerGW4Dg=
Xref: csiph.com comp.lang.java.programmer:15281

Well, it doesn't look "safe" to me because you are reading 
unsynchronized from a shared object in the logResults() method.  Always 
always always you must synchronized both the read and the write, or it's 
not thread safe.

You're also not MUTEX safe either, which clearly needs to be dealt with. 
  If you were using some form of Collections iterator, you'd probably be 
getting concurrent modification exceptions.

For the MUTEX/concurrent modification problem, I don't think using 
Collections.synchronizedMap() is enough.  You're going to have to block 
all access during the read.  This could ruin the concurrency of your 
app, so there are even further issues here to consider, but strictly 
from a thread safety issue it's the better solution I think.

void logResults(Map<String, Result> resultsMap) {
  StringBuilder logLine = new StringBuilder("Some stuff");
  synchronized( resultsMap ) {
    Set<String> keySet = resultsMap.keySet();
    for (String key: keySet) {
       logLine.append(key).append(": ").append(resultsMap.get(key));
    }
  }
  logger.info(logLine.toString());
}

Assuming "Some stuff" is thread safe.

However I think a different scheme altogether is needed.  Something like 
a ConcurrentLinkedQueue rather than a Map is probably an even better 
solution overall, from both an application performance & concurrency 
standpoint, and a thread safe & correct standpoint.

I realize you say your app is legacy.  I'm just tossing ideas out for 
future reference.