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.
