Closed
Bug 191739
Opened 22 years ago
Closed 22 years ago
Conn: not work if loopback is disabled
Categories
(Core :: Networking: HTTP, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.3beta
People
(Reporter: mkaply, Assigned: darin.moz)
Details
(Keywords: regression)
Attachments
(1 file)
18.73 KB,
patch
|
dougt
:
review+
bzbarsky
:
superreview+
asa
:
approval1.3b+
|
Details | Diff | Splinter Review |
As a result of 190000, the browser doesn't crash anymore but still doesn't work at all. It gives the error "http is not a registered protocol" Getting the protocol handler is failing in IOService. This only happens if loopback is disabled. I'm still investigating.
Reporter | ||
Comment 1•22 years ago
|
||
when loopback is disabled, querying the nsIProxiedProtocolHandler interface fails: http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsIOService.cpp#455
Reporter | ||
Comment 2•22 years ago
|
||
Did there used to be a backup to pollable events if they didn't work? This code in NSPR: http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/io/prsocket.c#1504 PR_InitializeNetAddr(PR_IpAddrLoopback, 0, &selfAddr); /* BugZilla: 35408 */ if (PR_Bind(listenSock, &selfAddr) == PR_FAILURE) { goto failed; checks for loopback and fails if it is not there. This is actually why mEventThread was null in 190000. So we have a MAJOR regression in the networking code that it doesn't support loopback. And the reason this happens on older Windows as well is because you used to be able to disable loopback.
Flags: blocking1.3b?
Keywords: regression
Comment 3•22 years ago
|
||
If nsIProxiedProtocolHandler fails, then whatever is calling it (i.e. calls nsIOService::NewChannelFromURI) should have code to handle a fail/NULL return code. Do you know what is calling this?
Reporter | ||
Comment 4•22 years ago
|
||
http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsNetUtil.h#163 which was called from: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#5227 which was called from: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#5145 which was called from: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#714 It's the very first http load in the browser.
Assignee | ||
Comment 5•22 years ago
|
||
ok, the solution of the old socket transport was to set a small timeout on PR_Poll and basically just busy wait. i had mistakenly thought that that was only required for Mac classic running with an older version of NSPR. i'll put together a similar patch for the new socket transport ASAP.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.3beta
Assignee | ||
Comment 6•22 years ago
|
||
this should do the trick.
Assignee | ||
Updated•22 years ago
|
Attachment #113438 -
Flags: superreview?(bzbarsky)
Attachment #113438 -
Flags: review?(dougt)
Reporter | ||
Comment 7•22 years ago
|
||
I can confirm that this fixes the problem. Thanks darin!
Assignee | ||
Comment 8•22 years ago
|
||
bz said he would be able to review this tomorrow. hopefully that is not too late for 1.3 beta.
Comment 9•22 years ago
|
||
Comment on attachment 113438 [details] [diff] [review] v1 patch r=dougt mike says it works.
Attachment #113438 -
Flags: review?(dougt) → review+
Comment 10•22 years ago
|
||
Comment on attachment 113438 [details] [diff] [review] v1 patch >+nsSocketTransportService::ServiceEventQ() >+{ >+ PRBool result; How about giving this a name that makes it clear what it does? Like "continueProcessing"? >+ if (!mThreadEvent) >+ NS_WARNING("running socket transport thread without a pollable event"); Replace that by NS_WARN_IF_FALSE(mThreadEvent, "running ...."); -- one less branch at runtime. ;) >+ // returns FALSE to stop processing the main loop >+ PRBool ServiceEventQ(); You mean PR_FALSE, right? >+ PRInt32 Poll(); Brief comment as to what this does?
Attachment #113438 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Comment 11•22 years ago
|
||
Comment on attachment 113438 [details] [diff] [review] v1 patch seeking drivers approval for 1.3 beta. this patch only looks big ;-) ... no seriously, most of the code changes are to a path that is never taken by most users.
Attachment #113438 -
Flags: approval1.3b?
Comment 12•22 years ago
|
||
Comment on attachment 113438 [details] [diff] [review] v1 patch This needs to land today if it's going to make 1.3. a=asa (on behalf of drivers) for checkin to 1.3beta.
Attachment #113438 -
Flags: approval1.3b? → approval1.3b+
Assignee | ||
Comment 13•22 years ago
|
||
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
bug 190000 is still fixed (some other way), right?
Comment 15•22 years ago
|
||
If I remember correctly, Bug 190000 was fixed via a NULL check on mThreadEvent, so I assume it's still fixed. You've changed the fix, but just by changing the returned message. Oh, by the way, did I see "if (!mThreadEvent)" within a "if (!mThreadEvent)" in your patch?
Comment 16•22 years ago
|
||
Today's build still has the problem ;(
Reporter | ||
Comment 17•22 years ago
|
||
Of course it does. He didn't check in until 1 in the afternoon. It will be in tomorrows build.
Comment 18•22 years ago
|
||
Who disables loopback? I can make this a testcase, but I'd need to know why.
Summary: Browser does not work if loopback is disabled → Conn: not work if loopback is disabled
Assignee | ||
Comment 19•22 years ago
|
||
ben: apparently it is fairly common on OS/2. mkaply verified the patch, see comment #7.
Reporter | ||
Comment 20•22 years ago
|
||
My understanding is that some proxy servers and firewalls do this as well (on Windows)
Comment 21•22 years ago
|
||
To add to comment #20. I just installed the latest nightly on WinXP, on launching Mozilla, ZoneAlarm (a software firewall) asked if I wanted to allow Mozilla to access IP address 127.0.0.1. If I said no, and this patch wasn't in, then Mozilla would have crashed either on startup or later on.
Updated•22 years ago
|
Flags: blocking1.3b?
Comment 22•21 years ago
|
||
VERIFIED per mike's comment. I've received some criticism for filing bugs about necko's standards compliance that are too essoteric in some people's minds, but running w/o a loopback hard for me to contemplate. Right now, I'm focused on basic unit testing and getting the verification list down. If I ever get to local firewall testing, I'll add this. David, what happens now that this is fixed? If there are any problems post-fix, please file a new bug and note it here.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•