Bug in mozilla/netwerk/base/src/nsAsyncStreamListener.h

VERIFIED FIXED in M14

Status

()

Core
Networking
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: duncan, Assigned: rpotts (gone))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
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: duncan@be.com
From: scc@netscape.com (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-----

Comment 1

18 years ago
I have no idead why this is firing... off to rpotts!
Assignee: gagan → rpotts

Updated

18 years ago
Target Milestone: M14

Comment 2

18 years ago
I'm attaching a patch to this.  Given this patch, you might want to reconsider 
whether |nsAsyncStreamListener::Init()| should still be inline.

Comment 3

18 years ago
Created attachment 5089 [details] [diff] [review]
patch fixing |nsAsyncStreamListener::Init()|'s COM type-error

Comment 4

18 years ago
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.

Comment 5

18 years ago
duncan, you could play with this patch and see how well it works for you...
(Assignee)

Comment 6

18 years ago
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
(Assignee)

Comment 7

18 years ago
Created attachment 5095 [details] [diff] [review]
Patch to fix the QI of the InputTestConsumer in TestSocketIO.cpp
(Reporter)

Comment 8

18 years ago
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.
(Assignee)

Comment 9

18 years ago
That's great!

I'll check it in once the tree opens.

thanks,
-- rick

Comment 10

18 years ago
I knew my patch would probably turn out to be superficial.  I'm glad you got to 
the bottom of this rick, thanks :-)
(Assignee)

Comment 11

18 years ago
I've finally checked the patch in.

-- rick
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 12

18 years ago
marking verified by reporter
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.