On Mon Mar 12 23:06:10 2012, Mike Mahoney wrote:
On Mar 10, 2012, at 6:45 PM, Niklas Gustavsson wrote:
On Wed, Mar 7, 2012 at 5:12 AM, Mike Mahoney<[email protected]> wrote:
I've been looking into Vysper-18, 'Recognize and act upon closing stream tag'.
I think I have a fix for it, but I wanted to get some input before doing a
commit. Here is what I'm doing:
1. I noticed that XMLParser successfully determines the end of the stream
document on lines 482-484, and notifies its content handler.
2. The Parser's content handler, XMPPContentHandler, ignores the endDocument()
call. Instead of just ignoring it, I modified the XMLElementListener interface
to be as follows:
public interface XMLElementListener {
void element(XMLElement element);
void close();
boolean isClosed();
}
and implemented this updated interface in XMPPDecoder.MinaStanzaListener. I
then gave the MinaStanzaListener a boolean closed field so it could track its
state.
3. In XMPPDecoder.doDecode() I modified the end of the method as follows:
XMPPContentHandler contentHandler = (XMPPContentHandler)
reader.getContentHandler();
XMLElementListener listener = new MinaStanzaListener(out);
contentHandler.setListener(listener);
reader.parse(in, CharsetUtil.UTF8_DECODER); // If parsing find the end of
the document, then the listener's close() will be called.
if (listener.isClosed()) {
session.close(false);
return true;
} else {
// we have parsed what we got, invoke again when more data is available
return false;
}
The call to session.close() then triggers the sessionClosed() handler in
XmppIoHandlerAdapter, which cleans up the Vysper SessionContext.
I'm particularly looking for any thoughts on the session closing. I'm not a
Mina expert by any means, so I want to make sure the close in
XMPPDecoder.doDecode() handles everything cleanly. I realize this doesn't
handle the BOSH case, but I can tackle that next.
I find it a bit weird to do session management in the XML decoding
code. Instead, could we not send a special marker (similar to
SslFilter.SESSION_SECURED) on the end of document. It could then be
detected in XmppIoHandlerAdapter where the session close would make
more sense I think. Do you think that would work?
This same thing occurred to me. My reasoning was that the end of document is
tightly tied to the end of the Mina session, so ending the mina session when
the end of the stream happens felt like it made sense. I have made some
additional changes to propagate the fact that it is a 'clean' client close up
to the XmppIoHandlerAdapter. I do think that doing the Vysper session cleanup
in the XmppIoHandlerAdapter makes sense.
BTW, we've been doing some scalability testing and this change makes a big
difference. If you have many clients going offline at the same time you will
see a lot of exceptions in the log, due to calls being made to the parser after
it is closed.
+1. great to see this basic XMPP feature finally being implemented.
thanks, Mike.
Don't hesistate to commit it, we're in Commit-Then-Review mode
Bernd
