On Mar 13, 2012, at 12:02 PM, Bernd Fondermann wrote: > 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 >
Thanks Bernd. I'll try to get it committed this week.
