Trunk builds cannot connect with the internet on Vista RC1 / UDP / LSP / Layered Service Provider

VERIFIED FIXED in 4.6.7

Status

P1
major
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: matthaeus123, Assigned: moco)

Tracking

({regression, verified1.8.0.12, verified1.8.1.4})

other
4.6.7
x86
Windows Vista
regression, verified1.8.0.12, verified1.8.1.4
Dependency tree / graph
Bug Flags:
blocking1.8.1.4 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [blocking as we need it along with bug #363997 which is a blocker])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
All trunk builds are incompatible with Vista RC1. They are all unable to connect with the internet. They seem like they are in offline mode.

Needed to reproduce:
Vista RC1, latest trunk build of Firefox or Thunderbird.

Steps to reproduce:
1. Start Firefox
2. Type URL in address bar.
3. press "enter" or green arrow to submit URL.
4. Repeat steps 2 and 3

What should have happened:
Firefox should have gone to the url submitted in the address bar.

What Happens:
Nothing. The throbber works for about 1 sec. and then nothing just a blank grey window.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-EN; rv:1.9a4pre) Gecko/20070330 Minefield/3.0a4pre
This sounds similar to bug 363997, though that bug should be fixed.
(Reporter)

Comment 2

12 years ago
Well, I have encountered it on the most recent versions of Thunderbird, and Sunbird.
Version: unspecified → Trunk
(Reporter)

Updated

12 years ago
Blocks: 376273
(Reporter)

Updated

12 years ago
Depends on: 363997
> Well, I have encountered it on the most recent versions of Thunderbird, and
> Sunbird.

Thunderbird / Sunbird trunk?

For firefox trunk, can you try a build before 2007-03-14  (which is when Wan-Teh landed the NSPR fix, see bug #363997 comment #40) and see if that works for you.

finally, Vista RC1?  Do you have access to the final version of Vista?  If so, do you see the problem there?

(Reporter)

Comment 4

12 years ago
I don't encounter the problem with this build...

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-EN; rv:1.9a3pre) Gecko/20070302 Minefield/3.0a3pre

I don't have access to the final version of Vista.
Carsten writes:

"I can confirm this Problem in Vista RC 1 (Build 5600)

I have done some Tests on this and:

-> Problem is on Minefield and on SeaMonkey Trunk
-> 1.8 Branch Nightlys (and Firefox 2.0.0.3) is fine

and i found a workaround :)
-> 1. Install Minefield as Admin
-> 2. Run Minefield in the XP Compatibility Mode

I have done this with the Compatibility Assistant and it was working fine.

However Vista RC1 is time bombed and will not working after the end of May."
Carsten, can you see my comment #3 about trying a build right before Wan-Teh landed?  (This will help confirm if that changed causes a problem with Vista RC 1).  Can you confirm that it works without running in the XP Compatibility Mode?
Seth, here is the regression range:

Works fine in
Mozilla/5.0 (Windows; U; Windows NT
6.0; en-US; rv:1.9a3pre) Gecko/2007031404
Minefield/3.0a3pre

Problems in
Mozilla/5.0 (Windows; U; Windows NT
6.0; en-US; rv:1.9a3pre) Gecko/2007031504 Minefield/3.0a3pre

So this regressed between 2007031404 and 2007031504

Keywords: regression
Carsten, thanks for pinpointing the regression range.  That range lines up directly with the change for bug #363997, which landed on 2007-03-14 11:04.

some follow up questions:

1)  what happens on Vista RTM, with a trunk build, if the user enables XP Compatibility Mode on Minefield?  will they see this bug?

2)  curious, with a trunk build before 2007031404, on Vista RC 1, can you successfully visit ultimateknicks.com without enabling XP Compatibility Mode on Minefield?  (bug #363997).
WSAIoctl will always fail with WSAEINVAL on Vista RC1 because RC1 didn't support SIO_SET_COMPATIBILITY_MODE.
Some non-Microsoft LSP may not support it, either. Per MSDN,
http://msdn2.microsoft.com/en-us/library/ms741621.aspx
> SIO_SET_COMPATIBILITY_MODE (opcode setting: I, T==3)
T==3 means:
> 3 The IOCTL applies only to a specific vendor's provider.

+        if (wsaioctlProc(sock, SIO_SET_COMPATIBILITY_MODE,  
+                         (char *)&mode, sizeof(mode),
+                         dummy, 4, &ret_dummy, 0, NULL) == SOCKET_ERROR)
+        {
+            PR_SetError(PR_UNKNOWN_ERROR, WSAGetLastError());
+            closesocket(sock);
+            return -1;
+        }
Why don't you just ignore the error?
> Why don't you just ignore the error?

kimura-san, that's a good point.  

To be honest, this is the first I've heard of LSP.  (http://en.wikipedia.org/wiki/Layered_Service_Provider)

I'm OK with silently failing there.  I'm thinking we should do something like:

+        if (wsaioctlProc(sock, SIO_SET_COMPATIBILITY_MODE,  
+                         (char *)&mode, sizeof(mode),
+                         dummy, 4, &ret_dummy, 0, NULL) == SOCKET_ERROR)
+        {
+          WSASetLastError(0);
+        }
+

i'll attach a patch for the trunk for kimura-san and wtc to review.

hopefully we can get it into the trunk of NSPR and tested on Vista RC 1 / Vista RTM, and then back ported to the appropriate NSPR branch for Fx 2.0.0.x
Assignee: nobody → sspitzer

Comment 11

12 years ago
It's not necessary to set the error code to 0 when a function
returns successfully.  The error code is only meaningful when
a function fails.

It seems that this WSAIoctl call may also fail for a UDP socket.
That'd be another reason to ignore its failure.
> It's not necessary to set the error code to 0 when a function
> returns successfully.  The error code is only meaningful when
> a function fails.

I agree, and so I'm planning to do:

if (wsaioctlProc(...) == SOCKET_ERROR)
{
  WSASetLastError(0);
}

Or did I mis-understand your comment?

> It seems that this WSAIoctl call may also fail for a UDP socket.
> That'd be another reason to ignore its failure.

Wan-Teh, can you suggest a way for us to confirm that?  (Perhaps with the nspr test, mozilla/nsprpub/pr/tests/socket.c)
Status: NEW → ASSIGNED
Created attachment 260737 [details] [diff] [review]
supplimental patch

patch.  on windows xp, I've forced this code to be executed in my local tree by doing:

     /* if Vista or later... */
-    if (osvi.dwMajorVersion >= 6)
+    if (osvi.dwMajorVersion >= 0)

This causes WSAIoctl() to fail, log the message (I have io:5 in my NSPR_LOG_MODULES), and proceed forward.

seeking review from both wtc and emk.
Attachment #260737 - Flags: review?(VYV03354)
Attachment #260737 - Flags: review?(wtchang)
Component: Networking → NSPR
Product: Core → NSPR
QA Contact: networking → nspr
Version: Trunk → other
adding more acronyms to the subject for future queries.
Summary: Trunk builds cannot connect with the internet on Vista RC1 → Trunk builds cannot connect with the internet on Vista RC1 / UDP / LSP / Layered Service Provider
Comment on attachment 260737 [details] [diff] [review]
supplimental patch

I confirmed this worked on Vista RC1.
> Or did I mis-understand your comment?
You don't have to clear the error code on success because nobody care about the value. It will be slightly faster to leave the error code.
Attachment #260737 - Flags: review?(VYV03354) → review+

Comment 16

12 years ago
Comment on attachment 260737 [details] [diff] [review]
supplimental patch

r=wtc.  Seth, please attach a new patch with these changes.  Thanks!

1. Don't use C++-style comment delimiters.

2. Don't bother to clear the error code.  The (NSPR or Windows) error
code is relevant only when _PR_MD_SOCKET fails.  It is fine for
_PR_MD_SOCKET to return successfully with a non-zero error code.

3. Change the outer if statement from

    if (socketSetCompatMode)

to (add parentheses to suit the style)

    if ((af == AF_INET || af == AF_INET6) && type == SOCK_STREAM && socketSetCompatMode)

This is because SIO_SET_COMPATIBILITY_MODE only applies to TCP sockets.

4. Nit: we may want to only allow WSAIoctl to fail with the WSAEINVAL error.
Attachment #260737 - Flags: review?(wtc) → review+
[blocking as we need it along with bug #363997 which is a blocker]
Flags: blocking1.8.1.4?
Whiteboard: [blocking as we need it along with bug #363997 which is a blocker]
Created attachment 261203 [details] [diff] [review]
revised supplimental patch, per wtc's comments (checked in on Pre 4.2 branch)
Attachment #260737 - Attachment is obsolete: true
Attachment #261203 - Flags: review?(wtc)

Comment 19

12 years ago
Comment on attachment 261203 [details] [diff] [review]
revised supplimental patch, per wtc's comments (checked in on Pre 4.2 branch)

r=wtc.  Please check it in on both the NSPR trunk and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH.  Thank you, Seth.
Attachment #261203 - Flags: review?(wtc) → review+
Wah-Teh, thanks for the review.

> Please check it in on both the NSPR trunk and the NSPRPUB_PRE_4_2_CLIENT_BRANCH

Is that message for me or Nelson?

Comment 21

12 years ago
Seth, could you check in the patch?  My CVS account isn't working now.
I've given you CVS commit access to the NSPR trunk and the NSPR client
branch.
Seth: NSPRPUB_PRE_4_2_CLIENT_BRANCH is what is pulled by trunk Mozilla, landing it there is a trunk fix but not a 1.8 branch fix.

On both the 1.8.0 and 1.8 branches we pull the NSPR_4_6_5_RTM tag. To include this fix on those branches either the NSPR team needs to create a snapshot tag for us (I doubt they want a _RTM tag for this, but could be wrong) or we need to take responsibility in the client team to create an unofficial snapshot.
dan:  thanks for the reminder.  I will take care of landing on the NSPR trunk and NSPRPUB_PRE_4_2_CLIENT_BRANCH.

after that lands, we should do some testing / baking before doing the release work for MOZILLA_1_8_BRANCH.  

as for the release work for a NSPR branch / tag:  nelson, is that something you could help me with?
Flags: blocking1.8.1.4? → blocking1.8.1.4+
(Reporter)

Comment 24

12 years ago
I am still encountering the problem with the latest trunk build.

Build ID:
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a4pre) Gecko/20070412 Minefield/3.0a4pre
> I am still encountering the problem with the latest trunk build.

matthaeus, the fix has not landed yet.
landed the fix on trunk and on the NSPRPUB_PRE_4_2_CLIENT_BRANCH branch.

Checking in w95sock.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/w95sock.c,v  <--  w95sock.c
new revision: 3.14; previous revision: 3.13
done

Checking in w95sock.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/w95sock.c,v  <--  w95sock.c
new revision: 3.7.4.7; previous revision: 3.7.4.6
done

I have not checked into the NSPR_4_6_BRANCH branch yet.  Nelson, can you help land this patch, and the others in bug #363997 to the NSPR_4_6_BRANCH branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Target Milestone: --- → 4.7
> WSAIoctl will always fail with WSAEINVAL on Vista RC1 because RC1 didn't
> support SIO_SET_COMPATIBILITY_MODE.
Sorry, this was wrong. WSAIoctl failed with WSAEOPNOTSUPP instead of WSAEINVAL.
So I couldn't connect with the internet on Vista RC1 yet.

I suggest that we don't check the error code at all.
http://msdn2.microsoft.com/en-us/library/ms679360.aspx
> The error codes returned by a function are not part of the Windows API 
> specification and can vary by operating system or device driver. For this
> reason, we cannot provide the complete list of error codes that can be
> returned by each function. There are also many functions whose documentation 
> does not include even a partial list of error codes that can be returned.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #261501 - Attachment mime type: application/octet-stream → text/plain
Created attachment 261627 [details] [diff] [review]
supplimental patch, per Kimura-san (checked in)
Attachment #261627 - Flags: review?(VYV03354)
Attachment #261627 - Flags: superreview?(wtc)
Comment on attachment 261627 [details] [diff] [review]
supplimental patch, per Kimura-san (checked in)

>             ** if the call to WSAIoctl() fails with WSAEINVAL
remove "with WSAEINVAL".
Attachment #261627 - Flags: review?(VYV03354) → review+

Comment 31

12 years ago
Comment on attachment 261627 [details] [diff] [review]
supplimental patch, per Kimura-san (checked in)

The comment needs to be updated.  Please change WSAEINVAL
to WSAEOPNOTSUPP in the comment when you check in this patch.

I would rather only tolerate a failure with specific error
codes (such as WSAEOPNOTSUPP), but I don't want this bug to
be bogged down by this minor issue.
Attachment #261627 - Flags: superreview?(wtc) → review+
Attachment #261203 - Attachment description: revised supplimental patch, per wtc's comments → revised supplimental patch, per wtc's comments (checked in on Pre 4.2 branch)
Comment on attachment 261627 [details] [diff] [review]
supplimental patch, per Kimura-san (checked in)

Committed on NSPRPUB_PRE_4_2_CLIENT_BRANCH
Checking in pr/src/md/windows/w95sock.c;
new revision: 3.7.4.8; previous revision: 3.7.4.7

Committed on trunk
Checking in pr/src/md/windows/w95sock.c;
new revision: 3.15; previous revision: 3.14

As checked in, the comment now reads:
            /* SIO_SET_COMPATIBILITY_MODE may not be supported.
            ** If the call to WSAIoctl() fails with WSAEOPNOTSUPP,
            ** don't close the socket.
            */
Attachment #261627 - Attachment description: supplimental patch, per Kimura-san → supplimental patch, per Kimura-san (checked in)
Confirmed cvs build worked on Vista RC1 build 5600.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Do you not want this fixed on the stable branch?
I want to land this (and bug 363997) on the branch.
I thought the "Status" field indicates a bug status about trunk. Is it wrong?
Hmm.  For NSS, target fix version is the first (earliest, lowest number) 
version in which the fix is expected, and status indicates the status for
the branch or trunk from which the target fix milestone will be released.
I thought it was the same for NSPR.  Maybe not?   Let's ask Wan-Teh.
So I guess this is fixed for NSPR, to get it fixed in Firefox we need to update our pull tag to some new NSPR tag when that gets created. Is there an intermediate tag we can pull for testing, or just the 4_2 branch?
Whiteboard: [blocking as we need it along with bug #363997 which is a blocker] → FF needs NSPR tag [blocking as we need it along with bug #363997 which is a blocker]
Blocks: 378958

Comment 38

12 years ago
I backported the patches to the NSPR_4_6_BRANCH for NSPR 4.6.7.
Target Milestone: 4.7 → 4.6.7
Keywords: fixed1.8.0.12, fixed1.8.1.4
Whiteboard: FF needs NSPR tag [blocking as we need it along with bug #363997 which is a blocker] → [blocking as we need it along with bug #363997 which is a blocker]
Marking P1, since it was handled that way.
Priority: -- → P1
Verified fixed on trunk with build Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a5pre) Gecko/2007043016 Minefield/3.0a5pre and Windows Vista RC1 Build : 5600 - the build is connecting succesfully to the internet on RC1 -> verified fixed 
Status: RESOLVED → VERIFIED
Nelson, Wan-Teh and Kimura-san:  thanks for finishing up this bug while I was away.

This bug has the following keywords:  fixed1.8.0.12, fixed1.8.1.4.

But i don't think it is fixed for fixed1.8.1.4 (fx 2.0.0.4) yet.  it has landed on the NSPR_4_6_BRANCH branch, but MOZILLA_1_8_BRANCH uses NSPR_4_6_5_RTM tag.

according to wtc's comment #38, at some point we'll need to switch mozilla/client.mk to use NSPR_4_6_7_RTM (or later).

as for fixed1.8.0.12, which implies fx 1.5.0.12, this fix will not land there, as the rest of the nspr / vista change are not part of the NSPR fixes used by the NSPR fixes used by the MOZILLA_1_8_0_BRANCH.

so I've removed those keywords.  right now, this is only fixed and verifiable on the mozilla firefox (and tbird) trunk.
Keywords: fixed1.8.0.12, fixed1.8.1.4
Seth, Wan-Teh (or someone) created an NSPR_4_6_7_BETA1 tag last week, 
and I believe/suspect that he did so precisely for mozilla to include in 
mozilla alpha/beta builds.  This tag is not suitable for mozilla official 
releases, IMO, but is suitable for mozilla pre-release testing.  

I don't know if that tag was announced anywhere.  I only learned about it
in an IM message from someone who was waiting for it.  
Per bug 378958, 1.8.1.4 and 1.8.0.12 now pull the NSPR_4_6_7_BETA1 tag.
Cc'ing bug 378958 reporter and patch author.
If all it takes to fix this one is to pull the NSPR patch then this should be fixed by bug 378958.
Keywords: fixed1.8.0.12, fixed1.8.1.4
kimura-san, dan and nick (via bug #378958), thanks for clarifying.  I see (locally) that we are using NSPR_4_6_7_BETA1 now (even though lxr is stale).

thanks for adding back the keywords.

Comment 46

12 years ago
Nelson, may I create the NSPR_4_6_7_RTM tag now?

Dan, Seth, you should take the NSPR_4_6_7_RTM tag when it's available.
Firefox 2.0.0.x releases should use NSPR RTM tags.
verified fixed 1.8.1.4 and 1.8.0.12 using >

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.4) Gecko/2007050903 Firefox/2.0.0.4 RC 2

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12 RC 2

Builds are working fine on Windows Vista RC 1

NSPR File Version is on both builds 4.6.7.0 - adding verified keywords
Keywords: fixed1.8.0.12, fixed1.8.1.4 → verified1.8.0.12, verified1.8.1.4
You need to log in before you can comment on or make changes to this bug.