[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