Release file handles back to caller on failure to take ownership

[PeterAdams-A]

Motivation:

When taking ownership of input and output file descriptors in a bootstrap the ownership of the file handles remains with the caller in the function taking ownership fails. This is currently where _takingOwnershipOfDescriptors uses NIOFileHandle to take ownership of descriptors but then experiences a later failure.

Modifications:

If an exception is throw in _takingOwnershipOfDescriptors release file descriptors from NIOFileHandle.

Result:

No crash on failure to close file handles before end of scoped usage.

[PeterAdams-A]

I can't think of a good way to unit test this. You can force a failure in the right place by passing in an already closed file handle as follows.

let sock = socket(AF_LOCAL, SOCK_STREAM, 0)
close(sock)

let channelFuture = NIOPipeBootstrap(group: NIOSingletons.posixEventLoopGroup).takingOwnershipOfDescriptor(inputOutput: sock)

Using a closed file descriptor of course is almost certain to result in a flaky test, especially if run in parallel.
Using numbers of file descriptors which are out of range or have never been used causes failure earlier in the call stack.

[PeterAdams-A]

@swift-server-bot test this please

[Lukasa]

The good news is that this is testable. The bad news is you need to use the Syscall Abstraction Layer to write the test. This will let you inject the EBADF you want to find, which will therefore also let you ensure the appropriate flow is hit.

[PeterAdams-A]

@Lukasa Let me know how you feel about the test I've tried adding. I looked into hooking sys calls but I believe the current sys call hooking doesn't touch this area at all yet. In order to hook the call made directly from PipeChannel I think I'd need to add a hooking mechanism to that which would involved changing the construction of that. Given the failure is from the construction of PipeChannel feels simpler to just throw the exception directly from a hooking factory for that.

Interested to hear your thoughts.

[Lukasa]

I don't hate this testing strategy, but it does need a little cleanup.

[Lukasa]

Suggested change

	outputPipe: NIOFileHandle?) throws -> PipeChannel;
	outputPipe: NIOFileHandle?) throws -> PipeChannel

[Lukasa]

Suggested change

	XCTAssertNotNil(bootstrap)
	XCTAssertNotNil(bootstrap)

[Lukasa]

I don't think we're using this socket anywhere.

[Lukasa]

Suggested change

	fileprivate struct DefaultNIOPipeBootstrapHooks : NIOPipeBootstrapHooks {
	fileprivate struct DefaultNIOPipeBootstrapHooks: NIOPipeBootstrapHooks {