http://d.puremagic.com/issues/show_bug.cgi?id=1478

           Summary: Please use threadsafe functions in getHostByName
           Product: D
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: default_357-line@yahoo.de


The following patch will change socket.d to use threadsafe functions for the
gethostbyname call on non-windows platforms. Not doing this can lead to all
sorts of ugly problems when using sockets in multi-threaded programs.
The patch is originally for gdc's socket.d, but should be equally applicable to
DMD.

diff socket.d.broken socket.d
487c487,497
<               hostent* he = gethostbyname(toStringz(name));
---
>               version (Windows)
>                       hostent* he = gethostbyname(toStringz(name));
>               else
>               {
>                       auto he = new hostent;
>                       auto buffer=new char[512];
>                       int errno;
>                       getHostByName_retry: // if we had extended do { } while { } this would not be necessary.
>                       auto res = gethostbyname_r(toStringz(name), he, buffer.ptr, buffer.length, &he, &errno);
>                       if (res == ERANGE) { buffer.length = buffer.length * 2; if (!he) he=new hostent; goto getHostByName_retry; }
>               }


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1478


braddr@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED




------- Comment #1 from braddr@puremagic.com  2007-10-13 20:06 -------
Given that the _r versions of the functions are glibc specific, I'm much more inclined to synchronize the call.

Additonally, this only protects one of the several non-threadsafe networking info api's.

Comitted this diff:

$ svn diff std/socket.d
Index: std/socket.d
===================================================================
--- std/socket.d        (revision 358)
+++ std/socket.d        (working copy)
@@ -453,7 +453,8 @@
         */
        bool getHostByName(string name)
        {
-               hostent* he = gethostbyname(toStringz(name));
+               hostent* he;
+                synchronized he = gethostbyname(toStringz(name));
                if(!he)
                        return false;
                validHostent(he);
@@ -468,7 +469,8 @@
        bool getHostByAddr(uint addr)
        {
                uint x = htonl(addr);
-               hostent* he = gethostbyaddr(&x, 4,
cast(int)AddressFamily.INET);
+               hostent* he;
+                synchronized he = gethostbyaddr(&x, 4,
cast(int)AddressFamily.INET);
                if(!he)
                        return false;
                validHostent(he);
@@ -485,7 +487,8 @@
        bool getHostByAddr(string addr)
        {
                uint x = inet_addr(std.string.toStringz(addr));
-               hostent* he = gethostbyaddr(&x, 4,
cast(int)AddressFamily.INET);
+               hostent* he;
+                synchronized he = gethostbyaddr(&x, 4,
cast(int)AddressFamily.INET);
                if(!he)
                        return false;
                validHostent(he);


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1478





------- Comment #2 from braddr@puremagic.com  2007-10-14 16:40 -------
downs:  I saw your comments in scrollback on irc.  Two comments:

1) thanks.. you're right about the need to globally synchronize and not per-class-instance synchronize.  I've fixed that:

- synchronized he = gethostbyname(toStringz(name));
+ synchronized(this.classinfo) he = gethostbyname(toStringz(name));

2) Regarding the use of static if, that would only work if there was sufficient configury mechanics to make sure that the _r versions were only included if they exist.  All static if can see is what's been declared, not what actually is linkable.  My main objection isn't really the declaration part of _r, but the very wierd usage of needing to grow that buffer and iterate many times. That'd likely end up more expensive than just synching.

Thanks for catching #1.

Later,
Brad


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1478


bugzilla@digitalmars.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED




------- Comment #4 from bugzilla@digitalmars.com  2007-11-03 21:46 -------
Fixed dmd 1.023


--