62877 - OS/2 changes to allow PSM to work

Status: VERIFIED FIXED

(NSPR :: NSPR, defect, P1)

Description

[jhp (no longer active)]

Several files need to be changed in NSPR to allow the PSM build to 
work; most importantly, that of setting the loopback device to be 
equal to 127.0.0.1.  Patch included below.

Comment 1

[jhp (no longer active)]

Attachment: OS/2 patch for nspr/pr files

Comment 2

[sobotka]

These patches appear to have bad side effects, at least with the OS/2 emx+gtk
build I'm presently working on. Normal page loads, and particularly images,
slowed down horrendously and at times failed completely. Backed them out and
everything was back to normal. If that doesn't happen with VACPP, perhaps wrap
them in ifdef XP_OS2_VACPP for now until I can have a look in the debugger.

Comment 3

[sobotka]

After culling all the cosmetic fixes from your patch, Javier, I end up with:

204c203
<     fd_set wd, ex;
---
>     fd_set wd;
229,231c227
<             FD_ZERO(&ex);
<             FD_SET(osfd, &ex);
<             rv = select(osfd + 1, NULL, &wd, &ex, tvp);
---
>             rv = select(osfd + 1, NULL, &wd, NULL, tvp);
241c237
<             rv = select(socks, 0, 1, 1, lTimeout);
---
>             rv = select(socks, 0, 1, 0, lTimeout);
246c242
<                if (FD_ISSET(osfd, &ex))
---
>                if (FD_ISSET(osfd, &wd))
256a254
>                    {
258,260d255
<                    else
<                        PR_SetError(PR_UNKNOWN_ERROR, 0);
<                    return -1;
262,265c257,259
<                if (FD_ISSET(osfd, &wd))
<                {
<                    /* it's connected */
<                    return 0;
---
>                    else
>                        return 0;   /* it's connected */
> 
266a261,262
>                else
>                    return -1;
267a264,266
>                if (socks[0] == osfd)
>                {
>                  len = sizeof(err);
273a273,281
> 
>                  if (err != 0)
>                  {
>                    _PR_MD_MAP_CONNECT_ERROR(err);
>                    return -1;
>                  }
>                  else
>                    return 0;   /* it's connected */
>                }
275c283
<                   return 0; /* It's connected ! */
---
>                  return -1;

Something in these 50-odd lines is most likely the culprit.

h~

Comment 4

[jhp (no longer active)]

the reason i changed those lines is because select (the bsd version, not the 
os/2 version) on os/2 sets the write bit if connect comes back with EWOULDBLOCK.  
however, on win32, select would set the exception bit in this case.  so this 
file was originally written by copying the win32 code, but it wasn't working 
since the exception bit never gets set on os/2 (at least , in this case).  so i 
removed the exception bit and just had the code check the write bit.  does emx 
use a different version of select?

Comment 5

[sobotka]

emx provides its own implementation of select() and a quick scan of the src 
shows nothing obvious to suggest bits aren't set as specified in the args. 
Also, EM is very explicit about any such limitations in his software and, and 
there's no mention of it in the docs. Since the problem seems confined to 
VAC++'s select(), just #ifdef your fix and let emx use the existing code.

Comment 6

[jhp (no longer active)]

Attachment: updated patch for os2sock.c re: henry's suggestions

Comment 7

[jhp (no longer active)]

Attachment: updated full nspr patch

Comment 8

[Dmitry Kubov]

Attachment: full files for EMX

Comment 9

[Dmitry Kubov]

this attach is .ZIP file with _os2.h, os2sock.c and os2poll.c

Comment 10

[jhp (no longer active)]

Attachment: latest patch with EMX changes, supercedes above patches

Comment 11

[Dmitry Kubov]

Attachment: improved EMX select(), nsprpub/pr/src/md/os2/os2poll.c

Comment 12

[Dmitry Kubov]

os2sock.c must be changed too: use XP_OS2_VACPP code for EMX too.
so, replace XP_OS2_VACPP by XP_OS2 or clean up it.

Comment 13

[Dmitry Kubov]

Attachment: patched os2sock.c (no cleanup)

Comment 14

[sobotka]

Confirming that the latest nspr patches (#24813, 25253, 25343) work fine here 
with an emx build of the 0.8 src. Thanks for the improved select(), Dimitry.

Comment 15

[Dmitry Kubov]

EMX-PSM is still not working on SMP machines... I can't fix this until someone
donate SMP system for me (hehe). VAC-PSM works on SMP.
Except SMP issue, everything works fine.

Comment 16

[Wan-Teh Chang]

Assigned the bug to Larry.

Which patches should we review and merge?

Comment 17

[sobotka]

#24813 for the two headers, 25253 and 25343 (the latter two override the rest 
of the first).

Comment 18

[jhp (no longer active)]

Thanks for answering that, Henry.  Does this all work now for EMX?

Comment 19

[sobotka]

Yes, both for Dimitry and here.

Comment 20

[Wan-Teh Chang]

Thanks.  I will review those patches, and Larry will pick up where
I left off next Monday.  (I'm going on sabbatical on Saturday.)

Comment 21

[Wan-Teh Chang]

In patch #24813, you added the following to _os2.h:
+#include <sys/builtin.h>
+#include <sys/smutex.h>
+static _smutex _md_shm_lock = 0;

Is this really necessary?  I don't see _md_shm_lock get used
anywhere. The reason I asked is that it is unusual to *define*
a static variable in a .h file as we will have an instance of
that static variable in every file that includes _os2.h.

Comment 22

[Wan-Teh Chang]

Attachment: A patch combining 24813 (for the two headers), 25253, and 25343.

Comment 23

[Wan-Teh Chang]

I checked in the combined patch (id=26597) on the tip and
NSPRPUB_CLIENT_BRANCH of NSPR.  Please verify that I merged
the patches correctly.

I'd also like to know if the following can be removed from _os2.h:
+#include <sys/builtin.h>
+#include <sys/smutex.h>
+static _smutex _md_shm_lock = 0;

Thanks!

Comment 24

[sobotka]

Yes. I don't see it used anywhere. Likely just something Dimitry was playing 
with.

Comment 25

[Wan-Teh Chang]

OK, I deleted those three lines of unused code from _os2.h.
Marked the bug fixed.

Comment 26

[jhp (no longer active)]

verified