`SessionConnector::startSessionTimer` causes thread leak on dynamic sessions

[Jallibad]

Describe the bug
AbstractSocketInitiator::createDynamicSession calls AbstractSocketInitiator::startInitiators which then calls SessionConnector::startSessionTimer. SessionConnector::startSessionTimer then creates a new scheduled SessionTimerTask check-in and overwrites the SessionConnector.sessionTimerFuture field with the ScheduledFuture<?> representing the session. When the AbstractSocketInitiator is eventually stopped cancel is called on that field. But if multiple dynamic sessions were added then only the last scheduled session timer is cancelled while all the others continue running.

To Reproduce
Create a SocketInitiator or ThreadedSocketInitiator and then call createDynamicSession multiple times.

Expected behavior
Calling AbstractSocketInitiator::createDynamicSession should not register a new session timer if one is already registered.

system information:

• OS: any
• Java version: any
• QFJ Version: I'm using 2.3.2 but this has likely been around for a while.

Additional context
Add any other context about the problem here.

[chrjohn]

@Jallibad , thanks for the bug report. Looking briefly at the code it seems you suggest that creating SessionTimerTask here should only be done once:

quickfixj/quickfixj-core/src/main/java/quickfix/mina/SessionConnector.java

Lines 313 to 321 in 9512fb7

	protected void startSessionTimer() {
	Runnable timerTask = new SessionTimerTask();
	if (shortLivedExecutor != null) {
	timerTask = new DelegatingTask(timerTask, shortLivedExecutor);
	}
	sessionTimerFuture = SCHEDULED_EXECUTOR.scheduleAtFixedRate(timerTask, 0, 1000L,
	TimeUnit.MILLISECONDS);
	log.info("SessionTimer started");
	}

[Jallibad]

Yeah exactly, I think if sessionTimerFuture is non-null and is not cancelled we shouldn't schedule another one