Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception handling in SocketReader.Read can leave Session in invalid state #471

Open
drweeto opened this issue Feb 23, 2018 · 0 comments
Open

Comments

@drweeto
Copy link

drweeto commented Feb 23, 2018

I've not managed to connect any of the existing issues in this repository to the scenario that I happened upon. I am using version 1.7.0.0.

Setup: one acceptor (using QuickFIXn); one initiator (not necessarily using QuickFIXn); one session.

As I understand it, there is the following interlinking relationship between Session, ClientHandlerThread and SocketReader:

  • Session has an IResponder (as a ClientHandlerThread) called responder_
  • ClientHandlerThread has a SocketReader called socketReader_
  • SocketReader has a Session called qfSession_ and a ClientHandlerThread called responder_

Say that I have an acceptor and an initiator connected and messages are being successfully sent and received. Then an error occurs that causes an exception to be thrown in SocketReader.Read on the acceptor side.

In SocketReader.Read, if a MessageParseError is thrown, the error is only passed to SocketReader.HandleExceptionInternal. For any other exception, the error is passed to SocketReader.HandleExceptionInternal and then thrown again. The re-thrown exception is caught in ClientHandlerThread.Run, which then triggers the ClientHandlerThread to be shutdown (in ThreadedSocketReactor.OnClientHandlerThreadExited).

If the exception that was re-thrown is not a SocketException, the ClientHandlerThread shutdown leaves the three interlinked objects in the following state:

  • Session: has a disposed IResponder (ClientHandlerThread) object. The object still exists, it is not null.
  • ClientHandlerThread: The "disposed" ClientHandlerThread has its SocketReader object set to null.
  • SocketReader: no longer exists.

A new ClientHandlerThread and a new SocketReader object have been created by this point. The SocketReader has its Session field (qfsession_) set to null.

The initiator still trying to send messages to the acceptor.

When the acceptor receives a message from the initiator, the message comes in to SocketReader.OnMessageFoundInternal. Because the SocketReader's Session object is set to null, Session.LookupSession is called. The session in found, so the message is passed to SocketReader.HandleNewSession.

In SocketReader.HandleNewSession, it is found that the session just located has a non-null ClientHandlerThread. This causes the following message to be logged and the message is left unhandled:

Multiple logons/connections for this session are not allowed (127.0.0.1:52978)

All the while, if attempts are still being made to send messages from the acceptor on the session, they are failing, because the SocketReader object of the trio no longer exists.

My question: apart from MessageParseErrors, which are handled differently, is it safe to do a session disconnection whenever any exception is thrown, rather than just when a SocketException is thrown?
Or is there something I am doing incorrectly with my use of the library?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant