Use close$NOCANCEL on the Mac

Use close$NOCANCEL on the Mac, so that it has deterministic behavior when
it fails with EINTR. Specifically, in this case, the FD will already have been
closed.

BUG=269623
R=thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223369

[Nico, 2013-09-14 00:15:25]

(I'm not ignoring this review, it just takes a while to process. I read through
the XNU sources of __pthread_testcancel(), unix_syscall_return(),
thread_exception_return() so far, I'm still reading more stuff)

https://codereview.chromium.org/23455051/diff/1/base/mac/close_nocancel.cc
File base/mac/close_nocancel.cc (right):

https://codereview.chromium.org/23455051/diff/1/base/mac/close_nocancel.cc#ne...
base/mac/close_nocancel.cc:24: // file implements an alternative: to use the
$NOCANCEL variant of close (this
s/\(this/(thus/?

[Nico, 2013-09-14 01:19:39]

So, do we have any evidence that closef_locked() actually can return EINTR?

Just looking through `ack EINTR ~/Downloads/xnu-2050.24.15/bsd/vfs/` it looks
like this only happens for open() and for reading buffers.

(In any case we'd like to make close() behave consistently on linux and os x, so
I'm now looking at the code.)

[Mark Mentovai, 2013-09-14 02:14:09]

The VFS layer passes the close on to a lower layer, which might be a filesystem
or something else. It’s maybe not possible to get a “late EINTR” out of HFS+,
but NFS is full of EINTRs, for example. There are also filesystem plug-ins that
aren’t in the xnu source. The FD need not be a file, either. Maybe it’s a
socket. If the close results in a buffer flush to the network in some way, then
it’s not unreasonable to expect that EINTR might be the eventual result. I
haven’t actually traced a complete path from closef_locked to late EINTR, but
I’m betting it’s possible.

No rush on the review if you want to take your time with it or anything. I’m
done for the week. Have a nice weekend.

[Nico, 2013-09-16 14:08:01]

https://codereview.chromium.org/23455051/diff/1/base/mac/close_nocancel.cc
File base/mac/close_nocancel.cc (right):

https://codereview.chromium.org/23455051/diff/1/base/mac/close_nocancel.cc#ne...
base/mac/close_nocancel.cc:33: // overridden.
Does this work without -flat_namespace / DYLD_FORCE_FLAT_NAMESPACE? What about
calls to close() from other bundles in our process (say, plugins)?

[Mark Mentovai, 2013-09-16 14:48:34]

Yes, it works without -flat_namespace.

It only affects our own code. It doesn’t impact other modules, which will have
bound to whatever variant of close in libsyscall they preferred.

I wouldn’t really want to affect other code. It’s incumbent on other libraries
to not be buggy. Ideally Apple would do something to just fix this for everyone,
but absent that, we should at least fix it for our own code.

[Nico, 2013-09-16 14:49:33]

On Mon, Sep 16, 2013 at 7:48 AM, <mark@chromium.org> wrote:

> Yes, it works without -flat_namespace.
>
> It only affects our own code. It doesn’t impact other modules, which will
> have
> bound to whatever variant of close in libsyscall they preferred.
>

Cool, that's what I thought. lgtm then.


> I wouldn’t really want to affect other code. It’s incumbent on other
> libraries
> to not be buggy. Ideally Apple would do something to just fix this for
> everyone,
> but absent that, we should at least fix it for our own code.
>

Did you file a  rdar?


>
>
https://codereview.chromium.**org/23455051/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mark Mentovai, 2013-09-16 16:56:12]

I filed a radar and updated the CL with a reference to it.