Inconsistency around Session's enabled attribute

[janusz-j]

Hello.

I'm playing around with quickfix and I'm trying to manage it via JMX using JConsole.
I observed inconsistency around the behaviour of the session logon/logoff operations depending if the session is an acceptor or an initiator.
When initiator session is connected and we issue a logoff() operation via JConsole the session logs off and stays disconnected, i.e. the initiator doesn't attempt to connect, until logon() is called again.
When we call logoff() for acceptor session, it issues Logout and disconnects the session, but immediately after that, it is ready to accept a connection from counterparty again.

When I looked at the code I found the enabled attribute of the Session, and the documentation for it says:
/*
* Controls whether it is possible to log on to this Session (if Acceptor)
* or if Logon is sent out respectively (if Initiator).
*/
private volatile boolean enabled;

That doesn't seem to be correct as far as I was able to test it.
Setting enabled to false, doesn't make it impossible to log into the acceptor session.

Could this be considered a bug?

If not, is there any way, to interactively set the acceptor session in "blocked" state, i.e. to disable accepting connections?

Regards,
Janusz.

[chrjohn]

Hi @janusz-j ,

I think this is a bug, yes. It has been around for quite a long time and I think most people work around it one way or the other.
Although it is not possible to disable accepting connections (which would require to unbind from the socket) you could prevent Logons by throwing RejectLogon exceptions in fromAdmin once a counterparty tries to logon at a time when you do not want to accept a Logon.

I don't know how much insight you have into the code but would it be possible for you to test a bug fix or create a PR for this?
At first sight I would say it would be sufficient to throw RejectLogon in this method when enabled is false and we are in Acceptor mode:

quickfixj/quickfixj-core/src/main/java/quickfix/Session.java

Line 2106 in 09d6104

private void nextLogon(Message logon) throws FieldNotFound, RejectLogon, IncorrectDataFormat,

Cheers,
Chris.

[janusz-j]

Hi Chris.

Thanks for your reply.
I think we need to distinguish two cases here for acceptor.

1. When Logout sequence is initiated locally (i.e. acceptor initiates it). In that case, session should stay blocked (enabled==false)
2. When Logout sequence is initiated by the counterparty. In that case, acceptor session should accept subsequent Logon attempts.

When browsing through the code I encounter the comment pointing to QFJ-457, which is handling similar problem, but I don't agree, that this is how it should work.
If the acceptor decides to logout, they should have a way to stay disconnected.
And obviously to accept inbound connection when they call logon().

I've created a PR for this issue: #360
Please let me know if you have any comments.

Regards,
Janusz.

[chrjohn]

Hi Janusz,
thanks for the PR, I think we can continue the discussion there.
Cheers,
Chris.

[chrjohn]

Hi @janusz-j ,
I gave this another thought and opened a follow-up issue to this. #390
What do you think?

Cheers,
Chris.

[tom-smalls]

Hi,

I know this has been around for a while but we just came across #360 when looking into changes in functionality on logoff.

We sometimes explicitly call logoff on a session in order to bounce it but this changes means that the session can no longer be bounced as it is disabled.

I was wondering if you were able to provide some context into why logoff and disabling a session were conflated? On first look, it feels like they are separate concepts but I am not experienced in this codebase so likely to be mistaken here. Secondly, how would you recommend logging off a session but allowing the initiator to logon again? The only thing I could think of is having a thread doing something like the following:

if (session.isLogoutSent() && !session.isEnabled()) {
  session.logon();
}

Thanks,

[chrjohn]

Session.logout() and logon() trigger an internal flag which is called isEnabled. This is a little misleading since logging off from a session does not disable it. Actually those methods should probably be called enable() and disable () or similar.
If you only want to logout the Session, use generateLogout().

[tom-smalls]

I'll take a look, thanks