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

Status: VERIFIED FIXED

(NSPR :: NSPR, defect, P1)

Description

[matthaeus123]

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

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This sounds similar to bug 363997, though that bug should be fixed.

Comment 2

[matthaeus123]

Well, I have encountered it on the most recent versions of Thunderbird, and Sunbird.

Comment 3

[(not reading, please use [e-mail] instead)]

> 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?

Comment 4

[matthaeus123]

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.

Comment 5

[(not reading, please use [e-mail] instead)]

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."

Comment 6

[(not reading, please use [e-mail] instead)]

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?

Comment 7

[Carsten Book [:Tomcat]]

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

Comment 8

[(not reading, please use [e-mail] instead)]

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).

Comment 9

[Masatoshi Kimura [:emk]]

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?

Comment 10

[(not reading, please use [e-mail] instead)]

> 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

Comment 11

[Wan-Teh Chang]

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.

Comment 12

[(not reading, please use [e-mail] instead)]

> 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)

Comment 13

[(not reading, please use [e-mail] instead)]

Attachment: 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.

Comment 14

[(not reading, please use [e-mail] instead)]

adding more acronyms to the subject for future queries.

Comment 15

[Masatoshi Kimura [:emk]]

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.

Comment 16

[Wan-Teh Chang]

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.

Comment 17

[(not reading, please use [e-mail] instead)]

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

Comment 18

[(not reading, please use [e-mail] instead)]

Attachment: revised supplimental patch, per wtc's comments (checked in on Pre 4.2 branch)

Comment 19

[Wan-Teh Chang]

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.

Comment 20

[(not reading, please use [e-mail] instead)]

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

[Wan-Teh Chang]

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.

Comment 22

[Daniel Veditz [:dveditz]]

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.

Comment 23

[(not reading, please use [e-mail] instead)]

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?

Comment 24

[matthaeus123]

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

Comment 25

[(not reading, please use [e-mail] instead)]

> I am still encountering the problem with the latest trunk build.

matthaeus, the fix has not landed yet.

Comment 26

[(not reading, please use [e-mail] instead)]

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.

Comment 27

[Masatoshi Kimura [:emk]]

> 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.

Comment 28

[Masatoshi Kimura [:emk]]

Attachment: NSPR log

Comment 29

[(not reading, please use [e-mail] instead)]

Attachment: supplimental patch, per Kimura-san (checked in)

Comment 30

[Masatoshi Kimura [:emk]]

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".

Comment 31

[Wan-Teh Chang]

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.

Comment 32

[Nelson Bolyard (seldom reads bugmail)]

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.
            */

Comment 33

[Masatoshi Kimura [:emk]]

Confirmed cvs build worked on Vista RC1 build 5600.

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

Do you not want this fixed on the stable branch?

Comment 35

[Masatoshi Kimura [:emk]]

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?

Comment 36

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 37

[Daniel Veditz [:dveditz]]

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?

Comment 38

[Wan-Teh Chang]

I backported the patches to the NSPR_4_6_BRANCH for NSPR 4.6.7.

Comment 39

[Nelson Bolyard (seldom reads bugmail)]

Marking P1, since it was handled that way.

Comment 40

[Carsten Book [:Tomcat]]

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 

Comment 41

[(not reading, please use [e-mail] instead)]

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.

Comment 42

[Nelson Bolyard (seldom reads bugmail)]

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.  

Comment 43

[Masatoshi Kimura [:emk]]

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.

Comment 44

[Daniel Veditz [:dveditz]]

If all it takes to fix this one is to pull the NSPR patch then this should be fixed by bug 378958.

Comment 45

[(not reading, please use [e-mail] instead)]

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

[Wan-Teh Chang]

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.

Comment 47

[Carsten Book [:Tomcat]]

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