[Issue 268] Bug with SocketSet and classes

[d-bugmail at puremagic.com]

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





------- Comment #12 from felipe.contreras at gmail.com  2007-07-15 19:29 -------
(In reply to comment #10)
> (In reply to comment #7)
> > The whole SocketSet class looks rather odd to me (given only a few minutes
> > looking at it).  It's confusing 'max' as a number of sockets that can be added
> > and the fact that the socket fd value won't necessarily have any relation to
> > that count.  Other oddities are functions like this:
> > 
> > socket_t* first() { return cast(socket_t*)buf; }
> > fd_set* _fd_set() { return cast(fd_set*)buf; }   
> > 
> > Treating the same block of memory as two different and incompatible things. 
> > Does this class even work as it's name suggests?
> > 
> > The proposed change to the constructor doesn't feel right given the arbitrary
> > value that a socket fd might have.  Using anything other than FD_SETSIZE for
> > 'max' is likely to result in problems.  NFDBITS is simply the number of bits
> > that fit in a word which doesn't make much sense as a minimum for nbytes.
> > 
> > Really, I'd just ditch the entire concept of a variable size set.  It doesn't
> > make any sense for unix style select() api's.  For other type of selection
> > interfaces it might make more sense such as epoll on linux, but there the
> > actual set is managed inside the kernel, not user space.
> > 
> 
> I agree, max should always be FD_SETSIZE.
> 
> /* Maximum number of file descriptors in `fd_set'.  */
> #define FD_SETSIZE              __FD_SETSIZE
> 

(In reply to comment #7)
> The whole SocketSet class looks rather odd to me (given only a few minutes
> looking at it).  It's confusing 'max' as a number of sockets that can be added
> and the fact that the socket fd value won't necessarily have any relation to
> that count.  Other oddities are functions like this:
> 
> socket_t* first() { return cast(socket_t*)buf; }
> fd_set* _fd_set() { return cast(fd_set*)buf; }   
> 
> Treating the same block of memory as two different and incompatible things. 
> Does this class even work as it's name suggests?
> 
> The proposed change to the constructor doesn't feel right given the arbitrary
> value that a socket fd might have.  Using anything other than FD_SETSIZE for
> 'max' is likely to result in problems.  NFDBITS is simply the number of bits
> that fit in a word which doesn't make much sense as a minimum for nbytes.
> 
> Really, I'd just ditch the entire concept of a variable size set.  It doesn't
> make any sense for unix style select() api's.  For other type of selection
> interfaces it might make more sense such as epoll on linux, but there the
> actual set is managed inside the kernel, not user space.
> 

I did some research.

The real problem is that FD_ZERO always erases the whole fd_set. Since the
fd_set didn't have the right size then more memory was erased and the
corruption generated the crash.

So we _need_ to have fd_set width the right size which turns out to be:
FD_SETSIZE / NFDBITS * int.size

That's because inside fd_set there isn't a list of sockets, but a binary map.
So if bit 0 is on, that means the socket with fd 0 should be checked and so on.
So the first() method was wrong, there's no way from the fd_set to find the
socket_t structure.

I'm not sure why a size of NFDBITS makes it not crash, maybe the zeroed
structures are not used in this case.

Anyway, I have tested the attached code with up to 1024 sockets. The assert
doesn't get activated because the socket creation fails.


--