534 bytes, patch
|Details | Diff | Splinter Review|
774 bytes, patch
|Details | Diff | Splinter Review|
I'm experiencing an assertion in nsCOMPtr::Assert_NoQueryNeeded() that is a consequence of the assignment at line 161 of mozilla/netwerk/base/src/nsAsyncStreamListener.cpp I noticed this problem when running TestSocketIO under BeOS, this is the stack crawl: loading symbols Break: at file ../../../dist/include/nsCOMPtr.h, line 446 _debugger: _debugger: +0007 ec084c03: * c3 retn TestSocketIO:sc frame retaddr fd000a68 ec72b7d0 nsDebug::Break(char const *, int) + 00000084 fd001248 ec72b73f nsDebug::Assertion(char const *, char const *, char const *, int) + 000000b7 fd001644 ea935762 .LSM155 + 00000032 fd001674 ea9356e4 .LSM146 + 00000006 fd00168c ea915830 nsAsyncStreamObserver::Init(nsIStreamObserver *, nsIEventQueue *) + 0000002c fd0016d8 ea935c87 nsAsyncStreamListener::Init(nsIStreamListener *, nsIEventQueue *) + 0000000b fd0016f8 ea93aa78 NS_NewAsyncStreamListener(nsIStreamListener *, nsIEventQueue *, nsIStreamListener **) + 00000038 fd001740 ea91b6a2 nsSocketTransport::AsyncRead(unsigned int, int, nsISupports *, nsIStreamListener *) + 0000021e fd0017a0 80003fcf TestWriteObserver::OnStopRequest(nsIChannel *, nsISupports *, unsigned int, unsigned short const *) + 00000077 fd0017cc ea915c7b nsOnStopRequestEvent::HandleEvent(void) + 000000b3 fd001800 ea915298 nsStreamListenerEvent::HandlePLEvent(PLEvent *) + 0000006c fd001820 ec720b6f PL_HandleEvent + 0000005b fd001838 ec7225e9 nsEventQueueImpl::HandleEvent(PLEvent *) + 00000079 fd001850 80004584 main + 00000564 fd001944 80003715 _start + 00000061 Scott Collins diagnosed this as a bug in nsAsyncStreamListener.h, below is his diagnosys, expressed better than I ever could hope to. Date: Mon, 7 Feb 2000 11:29:21 -0800 To: email@example.com From: firstname.lastname@example.org (Scott Collins) Subject: Re: Problem with nsCOMPtr on BeOS -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 OK. With the help of your stack trace, I found the problem. Look at line 93 of "nsAsyncStreamListener.h". You can see the problem: |nsAsyncStreamListener|s |Init| method takes an |nsIStreamListener*|, which it then passes down into |nsAsyncStreamObserver|s |Init| which takes an |nsIStreamObserver*|. |nsIStreamObserver*| !|nsIStreamListener*|. This is a bug. |nsAsyncStreamListener::Init| needs to QI to the right thing before calling down. You should file a bug on this. Hope this helps, ______________________________________________________________________ Scott Collins <http://ScottCollins.net> -----BEGIN PGP SIGNATURE----- Version: PGP Personal Privacy 6.0.2 Comment: get my key at <http://ScottCollins.net/#key> iQA/AwUBOJ8dDPGmojMuVn+fEQLiyACg6NsbKHR7xrKDzvKdK/P5qoLGTsQAoLuu /n22Kn6ShIdb3FblXneSRyk+ =K8wQ -----END PGP SIGNATURE-----
I have no idead why this is firing... off to rpotts!
I'm attaching a patch to this. Given this patch, you might want to reconsider whether |nsAsyncStreamListener::Init()| should still be inline.
Created attachment 5089 [details] [diff] [review] patch fixing |nsAsyncStreamListener::Init()|'s COM type-error
If this change is acceptable, I can just check in my patch. I don't really know, though, what the relationship between an |nsIStreamObserver| and an | nsIStreamListener| is; why there are two interfaces; or why I should expect that a listener also implements the observer API. And because I'm unsure... I didn't just check it in.
duncan, you could play with this patch and see how well it works for you...
hey scott, I believe that the real problem is that the necko test program,TestSocketIO, has incorrectly implemented it's QI for InputTestConsumer. The nsIStreamListener interface derives from nsIStreamObserver, so a nsIStreamListener is a valid interface to pass in as a nsIStreamObserver. However, the QI for InputTestConsumer (which implements nsIStreamListener) does *not* check for nsIStreamObserver :-( I'm attaching a patch which fixes the InputTestConsumer QI... -- rick
Created attachment 5095 [details] [diff] [review] Patch to fix the QI of the InputTestConsumer in TestSocketIO.cpp
scc's patch prevents crashes but doesn't really seem to work (TestSocketIO never exits). rpott's patch seems to work nicely here: no crash and good output.
That's great! I'll check it in once the tree opens. thanks, -- rick
I knew my patch would probably turn out to be superficial. I'm glad you got to the bottom of this rick, thanks :-)
I've finally checked the patch in. -- rick
marking verified by reporter