[Twisted-Python] [Twisted] #4747: tw.inet.test.ProcessTestsBuilder.test_openFileDescriptors is broken in many ways

Eric Faurot eric.faurot at gmail.com
Wed Nov 24 12:17:39 EST 2010


On Wed, Nov 24, 2010 at 4:17 PM,  <exarkun at twistedmatrix.com> wrote:
> On 02:56 pm, eric.faurot at gmail.com wrote:
>> So in my opinion, the function should really return the list of actually
>> opened files in all cases, and not fallback to a list of possible
>> list, otherwise the test case makes no sense.
>
> Maybe so.  Fortunately the problem really seems only to be with the _test_,
> right?  On OpenBSD the fallback case will really be used, and the file
> descriptors we want to close will be closed?

Well, actually no... On bsd, the /dev/* files are statically created. So as it
is now, it picks the 64 existing /dev/fd/* files, which is not correct
since higher
fd with a higher number will never be found. This is getting tricky. So if one
can't trust /dev/fd/ on some platform, maybe it should not be tried at all,
or we need to find a way to know at runtime/configure time if this is safe...

>> Of course this means trying all fds as last resort
>> on platforms that don't provide a simpler way to do it.  Doing it there or
>> doing it by ignoring EBADF on close() later would be the same, as far as
>> "suboptimization" is concerned.
>
> Right, and this is just what should happen on OpenBSD now.

Except for the problem above.

> Hopefully these are already marked as CLOEXEC, or lots of people would
> already be having lots of problems with them.  Despite that, they should be
> closed by `reactor.spawnProcess` (because that doesn't hurt and it's simpler
> than checking for CLOEXEC).  Then the only problem is that they are
> recreated in the child process and mess up the test's assumptions.
>
> Perhaps another approach to testing this function would be to create a file
> descriptor and assert that:
>
>  - That file descriptor is not open in the child process
>  - stdin, stdout, and sterr are open in the child process
>
> Only, without some care, one of the pthread pipe fds might end up looking
> like the probe descriptor, so maybe that can't really work.

That is closer to what the test is really about : spawnProcess don't leave
an undesired fd opened.

> closefrom() doesn't appear to help.  Descriptors intended to be kept open
> may be mixed together with descriptors which must be closed.  This is
> because of the `childFDs` parameter to `reactor.spawnProcess`, which allows
> arbitrary file descriptors to be passed to the child.

Right, I overlooked that.

Oh, I have just thought of another way to test all opened fds in just
one syscall.
It goes like this:

import resource
import select

_poll = [ 0, None ]

def listOpenFDs():
        maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[0]
        if maxfd != _poll[0]:
                _poll[0] = maxfd
                _poll[1] = select.poll()
                for fd in xrange(maxfd):
                        _poll[1].register(fd)
        return [ d for (d, r) in _poll[1].poll() if r != select.POLLNVAL ]


Eric.



More information about the Twisted-Python mailing list