[QFJ-823] Problem with concurrent access to UtcTimestampConverter#dateCache Created: 19/Jan/15 Updated: 02/Apr/15 Resolved: 22/Jan/15 |
|
| Status: | Closed |
| Project: | QuickFIX/J |
| Component/s: | None |
| Affects Version/s: | 1.5.3 |
| Fix Version/s: | 1.6.0 |
| Type: | Bug | Priority: | Default |
| Reporter: | Alexander Troshanin | Assignee: | Alexander Troshanin |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Attachments: |
|
| Description |
|
In quickfix library there is class private static HashMap<String, Long> dateCache As I see, this varaible is used as cache:
private static Long getMillisForDay(String value) {
String dateString = value.substring(0, 8);
Long millis = dateCache.get(dateString);
if (millis == null) {
Calendar c = new GregorianCalendar(1970, 0, 1, 0, 0, 0);
c.setTimeZone(SystemTime.UTC_TIMEZONE);
int year = Integer.parseInt(value.substring(0, 4));
int month = Integer.parseInt(value.substring(4, 6));
int day = Integer.parseInt(value.substring(6, 8));
c.set(year, month - 1, day);
millis = c.getTimeInMillis();
dateCache.put(dateString, c.getTimeInMillis());
}
return millis;
}
But if this code will be invoked from different threads(in my case it is so), then HashMap my become broken, => cuncurrent access and modification of hashmap. I would suggest to replace HashMap with ConcurrentHashMap private static ConcurrentHashMap<String, Long> dateCache = new ConcurrentHashMap<String, Long>(); |
| Comments |
| Comment by Christoph John [ 20/Jan/15 ] |
|
Hi, just a question: from where are you calling this concurrently? Does some of your code call UtcTimestampConverter.convert()? |
| Comment by Alexander Troshanin [ 20/Jan/15 ] |
|
Hi,
during this call UtcTimestampConverter.convert() is invoked, we dont call UtcTimestampConverter.convert() by ourselves. About a problems with hashmap, I did not get ConcurrentModificationException, but I got broken hashmap. It means that I cannot get elements by key from this map. When I call map.get(key), then this method never finishes it stucks in infinite loop. |
| Comment by Christoph John [ 20/Jan/15 ] |
|
I see, thanks for the clarification. Are you familiar with github? Would it be possible for you to open a pull request for this? Repo is here: https://github.com/quickfix-j/quickfixj |
| Comment by Alexander Troshanin [ 20/Jan/15 ] |
|
Yes, I have account in github: |
| Comment by Alexander Troshanin [ 21/Jan/15 ] |
|
I've crearted pull request. |
| Comment by Christoph John [ 21/Jan/15 ] |
|
Changes look OK to me, but you need to open the pull request against the quickfix-j repo (not against your own). Then I can merge it into the codebase. Edit: here is some help https://help.github.com/articles/creating-a-pull-request/ |