Code review comment for lp:~raharper/curtin/trunk.debug-mdadm

Revision history for this message
Ryan Harper (raharper) wrote :

On Mon, Jan 30, 2017 at 6:56 PM, Scott Moser <email address hidden> wrote:

> Heres an example on the case where hte os level file handle does not get
> closed.
> Whether its worth handling, I'll not comment. But it is a real
> possibility that you
> get an open file handle but the os.fdopen() fails, and thus the close
> never happens
> that would happen on the exit from the 'with'.
>
>
> #!/usr/bin/python3
> import os, sys
> path = sys.argv[1]
> with open(path, "w") as owritefp:
> owritefp.write("hello world\n")\
>

Not sure what this is doing here.

> # fd is an open file handle here.
> fd = os.open(path, os.O_RDONLY | os.O_EXCL)
>
> # this fails beacuse rb+ is not compatible with O_RDONLY above.
> # resulting in no 'close' being called on the "file object" fp.
> with os.fdopen(fd, "rb+") as fp:
> fp.write("bye bye\n")
>

> os.close(fd)
>

That's not what's happening;

 #!/usr/bin/python3
import os, sys, errno

path = sys.argv[1]

# fd is an open file handle here.
fd = os.open(path, os.O_RDONLY | os.O_EXCL)

# this fails beacuse rb+ is not compatible with O_RDONLY above.
# resulting in no 'close' being called on the "file object" fp.
print("fd=%s" % fd)
try:
    with os.fdopen(fd, "rb+") as fp:
        msg = "bye bye\n".encode("utf-8")
        fp.write(msg)
except Exception as e:
    print("Failed to write to fd: %s" % e)

print("closing fd:%s" % fd)
try:
    os.close(fd)
    print("close OK")
except OSError as err:
    if err.args[0] == errno.EBADF:
        print("fd isn't a valid file descriptor (man 2 close)")
    else:
        print("some other error")

This outputs:

% python3 fd.py /tmp/foo
fd=3
Failed to write to fd: [Errno 9] Bad file descriptor
closing fd:3
fd isn't a valid file descriptor (man 2 close)

I believe what happens is that the context manager constructs a FileObject
passing in the fd,
The FileObject exit hook handles closing the fd so that when we return from
it, the fd is
already closed.

In fact, that's _exactly_ what happens. If I drop the with and just use
assignments:

try:
    fp = os.fdopen(fd, "rb+")
    msg = "bye bye\n".encode("utf-8")
    fp.write(msg)
except Exception as e:
    print("Failed to write to fd: %s" % e)

And now the output looks like this:

 % python3 fd.py /tmp/foo
fd=3
closing fd:3
close OK

> --
> https://code.launchpad.net/~raharper/curtin/trunk.debug-
> mdadm/+merge/315805
> You are the owner of lp:~raharper/curtin/trunk.debug-mdadm.
>

« Back to merge proposal