[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: PNG File image.png    

 Description   

In quickfix library there is class
UtcTimestampConverter
in last version 1.5.3 there is static varaible 

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.
Under high load I can reproduce this on my dev server.

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()?
What is the problem you are seeing? Does it throw ConcurrentModificationException or are you not seeing all inserts to the map?

Comment by Alexander Troshanin [ 20/Jan/15 ]

Hi,
Right now in our application we have next architecture:

  • we receive fix message
  • save it in our database as text 
    public void onMessage(final quickfix.fix40.ExecutionReport executionReport, final SessionID sessionID) {
 ...
service.save(new FixMessage(id, executionReport.toString()));
  • then this fix message as text is retrieved from database by another module(this is separate JVM, actually there are several instances of this module) and converted to quickfix.fix40.ExecutionReport 
            ExecutionReport executionReport = new ExecutionReport();
        executionReport.fromString(messageString, dataDictionary != null ? dataDictionary : new DataDictionary("FIX40.xml"), false);
  • then we just call 
    executionReport.getTransactTime()

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.
In attach there is sceenshot of my debug

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:
troshanin

Comment by Alexander Troshanin [ 21/Jan/15 ]

I've crearted pull request.
It was my first time when I made pull request, so could you check if it is ok?

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.
Could you do me a favor and make the rest of the fields final, too? (utcTimestampConverter and dateCache) So we won't have a mixture of final and non-final fields.
Thanks

Edit: here is some help https://help.github.com/articles/creating-a-pull-request/
(I'm also not that familiar with github either)

Generated at Sat May 04 05:10:51 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.