Skip to content

Commit a6fa7f4

Browse files
alexwibowochrjohn
alexwibowo
authored and
chrjohn
committed
QFJ-964: Fixing a bug where sections inside SessionSettings is not th… (#216)
QFJ-964: Fixing a bug where sections inside SessionSettings is not threadsafe. * used ConcurrentHashMap (cherry picked from commit e695207)
1 parent 72635a6 commit a6fa7f4

2 files changed

Lines changed: 160 additions & 11 deletions

File tree

‎quickfixj-core/src/main/java/quickfix/SessionSettings.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package quickfix;
2121

22+
2223
import org.slf4j.Logger;
2324
import org.slf4j.LoggerFactory;
2425
import quickfix.field.converter.BooleanConverter;
@@ -36,7 +37,8 @@
3637
import java.net.UnknownHostException;
3738
import java.security.InvalidParameterException;
3839
import java.util.ArrayList;
39-
import java.util.HashMap;
40+
import java.util.concurrent.ConcurrentHashMap;
41+
import java.util.concurrent.ConcurrentMap;
4042
import java.util.HashSet;
4143
import java.util.Iterator;
4244
import java.util.List;
@@ -365,7 +367,7 @@ public void setBool(SessionID sessionID, String key, boolean value) {
365367
getOrCreateSessionProperties(sessionID).setProperty(key, BooleanConverter.convert(value));
366368
}
367369

368-
private final HashMap<SessionID, Properties> sections = new HashMap<>();
370+
private final ConcurrentMap<SessionID, Properties> sections = new ConcurrentHashMap<>();
369371

370372
public Iterator<SessionID> sectionIterator() {
371373
final HashSet<SessionID> nondefaultSessions = new HashSet<>(sections.keySet());

‎quickfixj-core/src/test/java/quickfix/SessionSettingsTest.java‎

Lines changed: 156 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package quickfix;
2121

22+
import org.junit.Test;
23+
2224
import java.io.ByteArrayInputStream;
2325
import java.io.ByteArrayOutputStream;
2426
import java.io.IOException;
@@ -28,15 +30,22 @@
2830
import java.util.Iterator;
2931
import java.util.Properties;
3032
import java.util.Set;
31-
32-
import junit.framework.TestCase;
33-
34-
public class SessionSettingsTest extends TestCase {
35-
36-
public SessionSettingsTest(String name) {
37-
super(name);
38-
}
39-
33+
import java.util.Map;
34+
import java.util.HashMap;
35+
import java.util.Random;
36+
import java.util.concurrent.CountDownLatch;
37+
import java.util.concurrent.atomic.AtomicBoolean;
38+
39+
import static org.junit.Assert.assertEquals;
40+
import static org.junit.Assert.assertTrue;
41+
import static org.junit.Assert.assertFalse;
42+
import static org.junit.Assert.assertNotNull;
43+
import static org.junit.Assert.assertNull;
44+
import static org.junit.Assert.fail;
45+
46+
public class SessionSettingsTest {
47+
48+
@Test
4049
public void testExtendedSettings() throws ConfigError {
4150
String settingsString = "";
4251
settingsString += "[SESSION]\n";
@@ -59,6 +68,7 @@ public void testExtendedSettings() throws ConfigError {
5968
assertEquals("NYC", id.getTargetLocationID());
6069
}
6170

71+
@Test
6272
public void testSettings() throws Exception {
6373
final SessionSettings settings = setUpSession();
6474

@@ -119,6 +129,7 @@ public void testSettings() throws Exception {
119129
assertFalse(sectionIterator.hasNext());
120130
}
121131

132+
@Test
122133
public void testMergedProperties() throws Exception {
123134
final SessionSettings settings = setUpSession();
124135
final SessionID sessionID = new SessionID("FIX.4.2", "TW", "CLIENT1");
@@ -172,6 +183,7 @@ private static SessionSettings createSettingsFromString(String settingsString)
172183
return new SessionSettings(cfg);
173184
}
174185

186+
@Test
175187
public void testSessionKeyIterator() throws Exception {
176188
final SessionSettings settings = setUpSession();
177189
final Iterator<SessionID> itr = settings.sectionIterator();
@@ -182,6 +194,7 @@ public void testSessionKeyIterator() throws Exception {
182194
}
183195
}
184196

197+
@Test
185198
public void testMethodsForDefaults() throws Exception {
186199
final SessionSettings settings = setUpSession();
187200
assertEquals("acceptor", settings.getString("ConnectionType"));
@@ -192,6 +205,7 @@ public void testMethodsForDefaults() throws Exception {
192205
assertFalse(settings.isSetting("bogus"));
193206
}
194207

208+
@Test
195209
public void testDefaultsSet() throws Exception {
196210
final SessionSettings settings = setUpSession();
197211
final Properties defaults = new Properties();
@@ -205,27 +219,31 @@ public void testDefaultsSet() throws Exception {
205219
assertEquals("bargle", settings.getString("FileStorePath"));
206220
}
207221

222+
@Test
208223
public void testSpecialCharactersInKeys() throws Exception {
209224
final SessionSettings settings = setUpSession("$$$foo bar.baz@@@=value\n");
210225
final SessionID sessionID2 = new SessionID("FIX.4.2", "TW", "CLIENT2");
211226

212227
assertEquals("value", settings.getString(sessionID2, "$$$foo bar.baz@@@"));
213228
}
214229

230+
@Test
215231
public void testStrangeCharactersInValues() throws Exception {
216232
final SessionSettings settings = setUpSession("label= This is a test? Yes, it is.\n");
217233
final SessionID sessionID2 = new SessionID("FIX.4.2", "TW", "CLIENT2");
218234

219235
assertEquals("This is a test? Yes, it is.", settings.getString(sessionID2, "label"));
220236
}
221237

238+
@Test
222239
public void testFinalCommentWithoutTrailingNewline() throws Exception {
223240
final SessionSettings settings = setUpSession("label=no trailing newline\n# a comment without trailing newline");
224241
final SessionID sessionID2 = new SessionID("FIX.4.2", "TW", "CLIENT2");
225242

226243
assertEquals("no trailing newline", settings.getString(sessionID2, "label"));
227244
}
228245

246+
@Test
229247
public void testDefaultSetters() throws Exception {
230248
final SessionSettings settings = setUpSession();
231249
settings.setBool("bool", true);
@@ -238,6 +256,7 @@ public void testDefaultSetters() throws Exception {
238256
assertEquals("wrong default value", "xyz", settings.getString("string"));
239257
}
240258

259+
@Test
241260
public void testVariableInterpolationWithDefaultValueSource() throws Exception {
242261
System.setProperty("test.1", "FOO");
243262
System.setProperty("test.2", "BAR");
@@ -251,6 +270,7 @@ public void testVariableInterpolationWithDefaultValueSource() throws Exception {
251270
}
252271

253272
// QFJ-204
273+
@Test
254274
public void testVariableInterpolationInDefaultSection() throws Exception {
255275
System.setProperty("sender", "SENDER");
256276
System.setProperty("target", "TARGET");
@@ -267,6 +287,7 @@ public void testVariableInterpolationInDefaultSection() throws Exception {
267287
assertEquals("wrong value", "TARGET", settings.getString(sessionID, "TargetCompID"));
268288
}
269289

290+
@Test
270291
public void testVariableInterpolationWithNoSysProps() throws Exception {
271292
System.setProperty("test.1", "FOO");
272293
System.setProperty("test.2", "BAR");
@@ -277,6 +298,7 @@ public void testVariableInterpolationWithNoSysProps() throws Exception {
277298
settings.getString("VariableTest"));
278299
}
279300

301+
@Test
280302
public void testVariableInterpolationWithProps() throws Exception {
281303
System.setProperty("test.2", "BAR");
282304
final Properties properties = new Properties(System.getProperties());
@@ -291,11 +313,13 @@ public void testVariableInterpolationWithProps() throws Exception {
291313
settings.getString("VariableTest"));
292314
}
293315

316+
@Test
294317
public void testDefaultConstructor() {
295318
new SessionSettings();
296319
// Passes if no exception is thrown
297320
}
298321

322+
@Test
299323
public void testConfigError() throws Exception {
300324
final InputStream cfg = new InputStream() {
301325

@@ -313,6 +337,7 @@ public synchronized int read() throws IOException {
313337
}
314338
}
315339

340+
@Test
316341
public void testSettingsToStream() throws Exception {
317342
final SessionSettings expectedSettings = setUpSession();
318343
final ByteArrayOutputStream out = new ByteArrayOutputStream();
@@ -345,12 +370,14 @@ private void assertSectionEquals(Properties expectedProperties, Properties actua
345370
}
346371
}
347372

373+
@Test
348374
public void testToString() throws Exception {
349375
final SessionSettings expectedSettings = setUpSession();
350376
final String actualString = expectedSettings.toString();
351377
assertSettingsEqual(expectedSettings, actualString);
352378
}
353379

380+
@Test
354381
public void testParseSettingReconnectInterval() {
355382
assertTrue(Arrays.equals(null, SessionSettings.parseSettingReconnectInterval("")));
356383
assertTrue(Arrays.equals(null, SessionSettings.parseSettingReconnectInterval(null)));
@@ -390,4 +417,124 @@ public void testParseSettingReconnectInterval() {
390417
}
391418
}
392419

420+
@Test
421+
public void testConcurrentAccess() throws ConfigError, InterruptedException {
422+
final Map<Object, Object> defaultSettings = createDefaultSettings();
423+
424+
final Map<Object, Object> pricingSection = createPricingSection();
425+
final SessionID pricingSessionID = new SessionID("FIX.4.2:FOOBAR_PRICING->*");
426+
427+
final Map<Object, Object> tradingSection = createTradingSection();
428+
final SessionID tradingSessionID = new SessionID("FIX.4.2:FOOBAR_TRADING->*");
429+
430+
final SessionSettings sessionSettings = new SessionSettings();
431+
sessionSettings.set(new Dictionary(null, defaultSettings));
432+
sessionSettings.set(pricingSessionID, new Dictionary("sessions", pricingSection));
433+
sessionSettings.set(tradingSessionID, new Dictionary("sessions", tradingSection));
434+
435+
436+
final int numClients = 500;
437+
final CountDownLatch startLatch = new CountDownLatch(1);
438+
final CountDownLatch countDownLatch = new CountDownLatch(numClients);
439+
440+
final AtomicBoolean testHasPassed = new AtomicBoolean(true);
441+
final Random random = new Random();
442+
443+
for (int i = 0; i < numClients; i++) {
444+
final String clientPricingSessionIDString = "FIX.4.2:FOOBAR_PRICING->CLIENT" + i;
445+
final String clientTradingSessionIDString = "FIX.4.2:FOOBAR_TRADING->CLIENT" + i;
446+
final Thread clientThread = new Thread(() -> {
447+
final Map<Object, Object> expectedClientPricingSettings = new HashMap<>();
448+
expectedClientPricingSettings.putAll(defaultSettings);
449+
expectedClientPricingSettings.putAll(pricingSection);
450+
451+
final Map<Object, Object> expectedClientTradingSettings = new HashMap<>();
452+
expectedClientTradingSettings.putAll(defaultSettings);
453+
expectedClientTradingSettings.putAll(tradingSection);
454+
455+
int randomSleep = random.nextInt(20);
456+
try {
457+
// wait for everyone to be ready
458+
startLatch.await();
459+
460+
// individual thread to sleep at random interval, to simulate spread connection attempt
461+
Thread.sleep(randomSleep);
462+
463+
final SessionID clientPricingSessionID = new SessionID(clientPricingSessionIDString);
464+
sessionSettings.set(clientPricingSessionID, new Dictionary(clientPricingSessionIDString, expectedClientPricingSettings));
465+
466+
final SessionID clientTradingSessionID = new SessionID(clientTradingSessionIDString);
467+
sessionSettings.set(clientTradingSessionID, new Dictionary(clientTradingSessionIDString, expectedClientTradingSettings));
468+
469+
// sleep at the end, before we verify the outcome
470+
Thread.sleep(randomSleep);
471+
472+
assertEquals("Default settings must be correct", defaultSettings, sessionSettings.get().toMap());
473+
assertEquals("Client pricing settings must be correct", expectedClientPricingSettings, sessionSettings.get(clientPricingSessionID).toMap());
474+
assertEquals("Client trading settings must be correct", expectedClientTradingSettings, sessionSettings.get(clientTradingSessionID).toMap());
475+
} catch (final Throwable throwable) {
476+
testHasPassed.set(false);
477+
throwable.printStackTrace();
478+
} finally {
479+
countDownLatch.countDown();
480+
}
481+
}, "CLIENT_THREAD_" + i);
482+
clientThread.setDaemon(true);
483+
clientThread.start();
484+
}
485+
486+
// go go go , everyone!
487+
startLatch.countDown();
488+
489+
// ok.. wait for everyone to finish
490+
countDownLatch.await();
491+
492+
// verify test has passed
493+
assertTrue(testHasPassed.get());
494+
}
495+
496+
private Map<Object, Object> createTradingSection() {
497+
final Map<Object, Object> tradingSection = new HashMap<>();
498+
tradingSection.put("PersistMessages","Y");
499+
tradingSection.put("SocketAcceptPort","7566");
500+
tradingSection.put("DataDictionary","fix/FIX42-TRADING-2.4.xml");
501+
tradingSection.put("ResetOnLogon","N");
502+
tradingSection.put("MaxLatency","1");
503+
return tradingSection;
504+
}
505+
506+
private Map<Object, Object> createPricingSection() {
507+
final Map<Object, Object> pricingSection = new HashMap<>();
508+
pricingSection.put("PersistMessages","N");
509+
pricingSection.put("SocketAcceptPort","7565");
510+
pricingSection.put("DataDictionary","fix/FIX42-PRICING-2.4.xml");
511+
pricingSection.put("ResetOnLogon","Y");
512+
pricingSection.put("MaxLatency","120");
513+
return pricingSection;
514+
}
515+
516+
private Map<Object, Object> createDefaultSettings() {
517+
final Map<Object, Object> defaultSettings = new HashMap<>();
518+
defaultSettings.put("TimeZone", "UTC");
519+
defaultSettings.put("StartDay", "Sunday");
520+
defaultSettings.put("StartTime", "7:00:00");
521+
defaultSettings.put("EndDay", "Friday");
522+
defaultSettings.put("EndTime", "17:00:00");
523+
defaultSettings.put("NonStopSession", "N");
524+
defaultSettings.put("ConnectionType", "acceptor");
525+
defaultSettings.put("HeartBtInt", "30");
526+
defaultSettings.put("UseDataDictionary", "Y");
527+
defaultSettings.put("ThreadModel", "ThreadPerSession");
528+
defaultSettings.put("UseJmx", "Y");
529+
defaultSettings.put("FileStorePath", "/home/wibowoa/var/lib/myApp");
530+
defaultSettings.put("FileLogPath", "logs/fixlog");
531+
defaultSettings.put("FileIncludeTimeStampForMessages", "Y");
532+
defaultSettings.put("FileIncludeMilliseconds", "Y");
533+
defaultSettings.put("CheckLatency", "Y");
534+
defaultSettings.put("BeginString", "FIX.4.2");
535+
defaultSettings.put("AcceptorTemplate", "Y");
536+
defaultSettings.put("TargetCompID", "*");
537+
return defaultSettings;
538+
}
539+
393540
}

0 commit comments

Comments
 (0)