quickfix-j / quickfixj

QuickFIX/J is a full featured messaging engine for the FIX protocol. - This is the official project repository.
http://www.quickfixj.org
Other
955 stars 611 forks source link

While looping issue #290

Open obrcn opened 4 years ago

obrcn commented 4 years ago

Hi, As I know, the messages is processed in the block method. If the application gets unexpected exception(nullpointer exception) or different exception in this method, the "while loop" can break and the messages can not process. I think, the "try catch" blocks should be cover whole "the while loop" block. So that when the application gets unexpected exception, "while loop" won't be broken and the messages is processed. This method can be like below.

private void block() {
    while (true) {
        try {
            synchronized (this) {
                if (isStopped) {
                    if (!eventQueue.isEmpty()) {
                        final List<SessionMessageEvent> tempList = new ArrayList<>(eventQueue.size());
                        queueTracker.drainTo(tempList);
                        for (SessionMessageEvent event : tempList) {
                            event.processMessage();
                        }
                    }
                    if (stopTime == 0) {
                        stopTime = SystemTime.currentTimeMillis();
                    }
                    if (!sessionConnector.isLoggedOn() || SystemTime.currentTimeMillis() - stopTime > 5000L) {
                        sessionConnector.stopSessionTimer();
                        // reset the stoptime
                        stopTime = 0;
                    }
                    return;
                }
            }

            SessionMessageEvent event = getMessage();
            if (event != null) {
                event.processMessage();
            }
            //Excepiton types can be seperate.
        } catch (Exception e) {
            Thread.currentThread().interrupt();
        }
    }
}

https://github.com/quickfix-j/quickfixj/blob/5a31fcd730829841d35a34919b351f487f288347/quickfixj-core/src/main/java/quickfix/mina/SingleThreadedEventHandlingStrategy.java#L89

the-thing commented 4 years ago

In theory you are right, but in practice there is no place in this method that can cause an exception to be thrown that is unhandled (as far as I can see).

You would have to pinpoint exact line and explain the circumstances under which this method can break the loop unexpectedly. If this is something that cannot be proven - we should not put try/catch blocks "just in case".