26506 - The DNS lock is not needed with native threads.

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

In prnetdb.c, there is a DNS lock that we hold
while copying the result of gethostbyname (and
its cousins getipnodebyname or gethostbyname2)
to our result buffer.  This is because traditionally,
gethostbyname returns its result in a static
buffer.

There are two ways to avoid this DNS lock.
1. Use the (non-standard) reentrant version of
   gethostbyname, gethostbyname_r.
2. If native threads are used and gethostbyname
   returns its result in a thread-specific data
   buffer (required by pthreads or some other
   POSIX or Unix standard), the result won't be
   overwritten by other threads' gethostbyname
   calls.

For these two cases, we should define LOCK_DNS()
and UNLOCK_DNS() to be null macros.

Comment 1

[Wan-Teh Chang]

I checked some common Unix flavors.  Here is
the result.

1. There are at least three flavors of gethostbyname_r.

2. Only OSF1 V4.0D, HP-UX 11.00, and AIX 4.3
   have a thread-safe gethostbyname() function
   that returns its result in thread-specific
   storage.  Error code is returned in the
   thread-specific h_errno on failure.

   The reentrant gethostbyname_r function on
   these platforms is deprecated and new code
   is advised to use the thread-safe gethostbyname
   function.

3. Solaris 2.5.1 - 8 and IRIX 6.5 (but not
   IRIX 6.3) have gethostbyname_r with this
   function prototype:
     struct hostent *gethostbyname_r(
         const char *name,
         struct hostent *result,
         char *buffer,
         int buflen,
         int *h_errnop);
   If the buffer is too small, errno is set to ERANGE.
   Otherwise, error code is returned in *h_errnop on
   failure.

   Note that the gethostbyname(3NSL) man page on Solaris
   8 says the following in the "WARNINGS" section:
     The reentrant interfaces gethostbyname_r(),
     gethostbyaddr_r(), and gethostent_r() are included
     in this release on an uncommitted basis only, and
     are subject to change or removal in future minor
     releases.
   But the gethostbyname(3N) man page on Solaris 2.5.1
   says the same thing.

4. RedHat Linux 6.0 and 6.1 have a gethostbyname_r
   function that works but is not documented.  It
   has a different function prototype:
     int gethostbyname_r(
         const char *name,
         struct hostent *result_buf,
         char *buf,
         size_t buflen,
         struct hostent **result,
         int *h_errnop);

Is it worth the trouble to use gethostbyname_r
on Solaris, IRIX, and Linux, in light of the
warning in the Solaris man page and the fact
that it is not documented on Linux?

Comment 2

[Wan-Teh Chang]

One thing that's easy to fix: the getipnodebyname
and getipnodebyaddr functions of RFC 2553 are
thread safe, so it's not necessary to lock around
them.
/cvsroot/mozilla/nsprpub/pr/src/misc/prnetdb.c, revision 3.15

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch.

Comment 4

[Wan-Teh Chang]

The proposed patch (id=9114) introduced the following
new configuration macros:
1. _PR_HAVE_THREADSAFE_GETHOST: define this macro if
   the gethostbyXXX functions return the result in
   thread-specific storage.  E.g., AIX, HP-UX, and OSF1.
2. _PR_HAVE_GETHOST_R: define this macro if the platform
   has the reentrant gethostbyXXX_r functions.  See
   the next two macros.
3. _PR_HAVE_GETHOST_R_POINTER: define this macro if the
   gethostbyXXX_r functions return a struct hostent* pointer.
   E.g., IRIX and Solaris.
4. _PR_HAVE_GETHOST_R_INT: define this macro if the
   gethostbyXXX_r functions return an int.  E.g., Linux
   glibc.

We don't need to use the DNS lock if
_PR_HAVE_THREADSAFE_GETHOST or _PR_HAVE_GETHOST_R is
defined.  I've only checked the six Unix flavors
mentioned above.  Other Unix flavors still need to
be checked.

I checked in the patch on the main trunk.
/cvsroot/mozilla/nsprpub/config/AIX.mk, revision 3.8
/cvsroot/mozilla/nsprpub/config/HP-UX.mk, revision 3.9
/cvsroot/mozilla/nsprpub/config/IRIX.mk, revision 3.11
/cvsroot/mozilla/nsprpub/config/OSF1.mk, revision 3.11
/cvsroot/mozilla/nsprpub/pr/include/md/_linux.h, revision 3.27
/cvsroot/mozilla/nsprpub/pr/include/md/_solaris.h, revision 3.13
/cvsroot/mozilla/nsprpub/pr/src/misc/prnetdb.c, revision 3.17

Gordon, the best way to test this patch is to check out the
tip of mozilla/nsprpub.

Comment 5

[Wan-Teh Chang]

It turns out that the gethostbyXXX_r functions in
glibc on Linux are documented in 'info libc'; they
are just not documented in the man pages.  On a
RedHat Linux 6.1 machine, fire up 'info libc', press
's' (to search for string), and type 'gethostbyname_r'.

Comment 6

[Wan-Teh Chang]

Marked the bug fixed.

Note that the fix was only checked in on the
tip of NSPR.  Because Mozilla is pulling the
NSPRPUB_CLIENT_BRANCH of NSPR, it is not picking
up this fix right now.