Closed
Bug 143311
Opened 22 years ago
Closed 22 years ago
nsStreamListenerTee should completely drop mSink during OnStopRequest
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
People
(Reporter: darin.moz, Assigned: darin.moz)
Details
(Whiteboard: [fixed trunk])
Attachments
(1 file)
7.11 KB,
patch
|
dougt
:
review+
rpotts
:
superreview+
|
Details | Diff | Splinter Review |
nsStreamListenerTee should completely drop mSink during OnStopRequest... this code: NS_IMETHODIMP nsStreamListenerTee::OnStopRequest(nsIRequest *request, nsISupports *context, nsresult status) { NS_ENSURE_TRUE(mListener, NS_ERROR_NOT_INITIALIZED); mInputTee = 0; mSink = 0; return mListener->OnStopRequest(request, context, status); } does not do enough... it needs to also ensure that mInputTee also drops its reference to mSink. dougt noticed this bug when he tried to fetch a HTTP URL via javascript initiated from a timer callback... apparently, javascript garbage collection only runs when we load a page. as a result, the reference that dougt's code held to mInputTee (for reading the data in his OnDataAvailable) was never getting released (garbage collected). sure, this should probably be solved in the JS GC code, but we clearly have a bug in necko as well, since it is perfectly valid to hold a reference to the input stream given out in OnDataAvailable. patch in hand...
Assignee | ||
Comment 1•22 years ago
|
||
this patch changes nsIInputStreamTee to make it possible to drop the sink. gordon: i've also added the error handling code we talked about adding to TeeSegment. if mSink->Write fails, we simply drop mSink, and pretend that nothing bad happened.
Comment 2•22 years ago
|
||
I solved this by explictly calling close() on the scriptable input stream wrapping the inputstream passed by oda.
Comment 3•22 years ago
|
||
Comment on attachment 82941 [details] [diff] [review] v1 patch this works great for me! r=dougt
Attachment #82941 -
Flags: review+
Comment on attachment 82941 [details] [diff] [review] v1 patch r=gordon as well. Thanks for adding the error handling code for mSink->Write().
Comment 5•22 years ago
|
||
Comment on attachment 82941 [details] [diff] [review] v1 patch sr=rpotts@netscape.com
Attachment #82941 -
Flags: superreview+
Assignee | ||
Comment 6•22 years ago
|
||
fixed-on-trunk
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [fixed trunk]
Comment 7•22 years ago
|
||
Will this stop XMLHttpRequest from continuing even after a page reload? You can see this behavior at: http://www.visi.com/~hoju/humor.html Click the "Check Links" button and it will begin looping through all the links on the page and make HEAD requests to the links and then reports their status by inserting the returned HTTP status code right after the link. What I am seeing is that if I start this process, do a shift+reload of the page, the script continues on in a loop from where it last left off (without ever pressing the "Check Links" button again). In the status bar, it tells you which link it is currently processing. If it was "checking link 5" right before I did the reload, after the page reloads, it starts back up again at "checking link 6". I would have expected that the whole script would have started over since the scope of the script should have been lost upon reload of the page. Am I wrong here? Does the fix for this bug fix this issue? If this is a different bug, is it a known bug or should it be reported as a new one? BTW, you will need to add the following to your prefs.js for this to work: user_pref("signed.applets.codebase_principal_support", true); Then, when asked whether to allow this script to run, check "remember decision" and click "Ok". Jake
Assignee | ||
Comment 8•22 years ago
|
||
jake: that sounds like an unrelated issue to me... but try it out for yourself now that this patch has landed on the trunk.
You need to log in
before you can comment on or make changes to this bug.
Description
•