www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 1478] New: Please use threadsafe functions in getHostByName

reply d-bugmail puremagic.com writes:
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; }
               }



--
Sep 06 2007
next sibling parent d-bugmail puremagic.com writes:
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);


--
Oct 13 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
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


--
Oct 14 2007
prev sibling parent d-bugmail puremagic.com writes:
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


--
Nov 03 2007