CookieStoreTest.testConcurrentLoad fails randomly

[ato]

Just hit this with a Travis on #271. It looks like someone else also hit when building from source in #175.

testConcurrentLoad(org.archive.modules.fetcher.CookieStoreTest)  Time elapsed: 1.188 sec  <<< FAILURE!
junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at junit.framework.Assert.assertTrue(Assert.java:27)
	at org.archive.modules.fetcher.CookieStoreTest.testConcurrentLoad(CookieStoreTest.java:391)

The problem seems to have been introduced by #133:

I don't think we need the synchronization in fact. Because the 50 cookie limit doesn't have to be completely strict. It's just a cutoff to avoid problems, and if a domain has 51 cookies, or even 60, it's unlikely to be a problem. So my suggestion is to remove the synchronization, and change CookieStoreTest:350 to something like assertTrue(domainCounts.get(domain) <= BdbCookieStore.MAX_COOKIES_FOR_DOMAIN + 5);, i.e. check that it's not way over the limit.

Unfortunately I don't think this assumption is safe. Not just because it can cause the tests to randomly fail either. At the NLA we've actually hit a gotcha in several different applications where Java's HashMap goes into an infinite loop when accessed concurrently without synchronization.

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6423457

If a HashMap is used in a concurrent setting with insufficient synchronization,
it is possible for the data structure to get corrupted in such a way that
infinite loops appear in the data structure and thus get() could loop forever.

Edit: Confused myself reading the code. The HashMap is not accessed in multiple threads.