I hope we get an answer from the core team as it seems something they must have 
considered already. Sounds serious.

Sent from my iPhone

> On 4 Nov 2016, at 08:12, Johannes Weiß via swift-evolution 
> <[email protected]> wrote:
> 
> Hi,
> 
> I just realised, that the problem is slightly worse than I originally 
> described. I believed that successful calls in between the actual call the 
> programmer wanted to make and capturing `errno` are not a problem.
> 
> But POSIX seems to suggest [4] that "The setting of errno after a successful 
> call to a function is unspecified unless the description of that function 
> specifies that errno shall not be modified." . The Linux man page [5] also 
> mentions that "a function that succeeds is allowed to change errno."
> 
> To me this means that the issue is wider than just ARC. I think the problem 
> extends to memory allocations on the heap. Failed memory allocations aren't a 
> problem because they are terminal in Swift. However, _successful_ memory 
> allocations might be a problem because the malloc(3) that the Swift compiler 
> will use is absolutely free to set errno to 0 (or any other value in fact) 
> indicating success. (Said that at least malloc doesn't change `errno` on the 
> macOS or Linux I tested today, we probably shouldn't rely on that though.)
> 
> This makes it even more unpredictable to the programmer what a use of `errno` 
> in Swift will return. IMHO it shouldn't be exported to Swift as its value is 
> undefined almost(?) everywhere.
> 
> [4]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html
> [5]: http://man7.org/linux/man-pages/man3/errno.3.html
> 
> All the best,
>  Johannes
> 
>> On 2 Nov 2016, at 1:12 pm, Johannes Weiß via swift-evolution 
>> <[email protected]> wrote:
>> 
>> Hey swift-evolution,
>> 
>> First of all apologies, this is not a full proposal yet, it's meant to kick 
>> off a discussion on how to resolve the issue.
>> 
>> # Make `errno`-setting functions more usable from Swift
>> 
>> ## Introduction
>> 
>> This is a pitch to make [`errno`][1]-setting functions properly usable, as 
>> in having a guarantee to get the correct `errno` value on failure of a 
>> [system call][2]. Currently, functions which set `errno` are just exported 
>> in the Darwin/Glibc modules with (as far as I understand) no guaranteed 
>> correct way of handling errors as the correct `errno` value can't be 
>> retrieved.
>> This means that much of the Swift code which uses Darwin/Glibc out there 
>> relies on behaviour that isn't guaranteed.
>> 
>> 
>> ## Motivation
>> 
>> In many Swift libraries that use the Darwin/Glibc modules there is code 
>> similar to:
>> 
>> ```
>> /* import Darwin/Glibc */
>> 
>> let rv = some_system_call(some, parameters)
>> if rv < 0 {
>>  throw SomeError(errorCode: errno) /* <-- errno use */
>> }
>> ```
>> 
>> That looks very innocent but please note that `errno` is used here. And 
>> `errno` is an interesting one as it's a thread-local variable which is 
>> written to by many functions. A thread-local variable is like a global 
>> variable except that setting it in one thread does not affect its value in 
>> any other thread. Pretty much all system calls and many library functions 
>> set `errno` if something went wrong.
>> 
>> The problem is that as far as I see (and Swift developers have confirmed), 
>> there is no guarantee that in between the call of `some_system_call` and the 
>> reading of `errno`, `errno` hasn't been overwritten by some other system 
>> call that has been call on the same thread.
>> 
>> To illustrate this further, let's consider this example
>> 
>> ```
>> /* import Darwin/Glibc */
>> public class SomeClass {
>> public let someValue: Int = 1
>> deinit {
>>     /* call some failing syscall, for example */
>>     write(-1, nil, 0) /* should set errno to EBADF */
>> }
>> }
>> 
>> public func foo() {
>>  let x = SomeClass()
>>  let rv = write(x.someValue, nil, 0)
>>  let errnoSave = errno
>>  if rv != 0 {
>>     throw SomeError(errorCode: errnoSave)
>>  }
>> }
>> ```
>> 
>> as you see in function `foo`, the instance `x` of `SomeClass` isn't needed 
>> anymore as soon as `write` has been called. So (as far as I understand) 
>> there's no guarantee that ARC doesn't turn the above code into
>> 
>> ```
>> let x = SomeClass()
>> let rv = write(x.someValue, nil, 42) /* should set errno to EFAULT */
>> /* ARC generated */ x.release()
>> let errnoSave = errno /* wrong errno value :( */
>> if rv != 0 {
>> throw SomeError(errorCode: errnoSave)
>> }
>> ```
>> 
>> And the ARC generated `x.release()` will cause `x` to be deallocated which 
>> will call the failing `write` in the `deinit` of `SomeClass`. So `errnoSave` 
>> might be `EBADF` instead of `EFAULT` depending on where ARC put the 
>> `x.release()` call.
>> 
>> What `errno` value we read will depend on the optimisation settings and the 
>> Swift compiler version. That's IMHO a big issue as it might make the lowest 
>> layers unstable with hard-to-debug issues.
>> 
>> 
>> ## Proposed solution
>> 
>> I don't have a full story on how to actually resolve the issue but I see a 
>> few options:
>> 
>> ### Option 1: always return errno
>> 
>> clang importer could be changed to make all `errno`-setting functions return 
>> a tuple of the actual return value and the `errno` value.
>> 
>> For example, currently write(2) is imported as:
>> 
>> ```
>> public func write(_ __fd: Int32, _ __buf: UnsafeRawPointer!, _ __nbyte: Int) 
>> -> Int
>> ```
>> 
>> which could be changed to
>> 
>> ```
>> public func write(_ __fd: Int32, _ __buf: UnsafeRawPointer!, _ __nbyte: Int) 
>> -> (Int, Int32 /* for errno */)
>> ```
>> 
>> Correct code to use write would then look like this:
>> 
>> ```
>> let (bytesWritten, writeErrno) = write(fd, buf, len)
>> if bytesWritten >= 0 {
>>  /* everything's fine */
>> } else {
>>  throw POSIXError(code: writeErrno)
>> }
>> ```
>> 
>> 
>> ### Option 2: make them throw
>> 
>> The second option is to teach clang importer to make the functions throwing. 
>> So write(2) would be imported as
>> 
>> ```
>> public func write(_ __fd: Int32, _ __buf: UnsafeRawPointer!, _ __nbyte: Int) 
>> throws /* POSIXError */ -> Int
>> ```
>> 
>> That would make these functions quite easy to use and would feel natural in 
>> Swift:
>> 
>> ```
>> do {
>>  let bytesWritten = write(fd, buf, len)
>> } catch let e as POSIXError {
>>  /* handle error */
>> } catch {
>>  ...
>> }
>> ```
>> 
>> 
>> ### Discussion
>> 
>> The beauty of option 1 is simplicity. Clang importer would not need to know 
>> what exact values a system call returns on failure. Also very little 
>> additional code needs to be emitted for calling a system call. That seems to 
>> be the [way Go is going][3].
>> 
>> The downside of option 1 is that the API doesn't feel like idiomatic Swift. 
>> The returned `errno` value is only useful if the system call failed and is 
>> arbitrary in the case when it worked. (There is no guarantee that `errno` is 
>> set to `0` when a system call succeeds.)
>> Also there is a slight overhead in reading `errno` which would be paid for 
>> every `errno`-setting function, even if successful. Hence, option 2 looks 
>> nice as it brings these functions more in like with other Swift functions. 
>> However, as mentioned before, clang importer would need to learn what values 
>> are returned on success/failure for _every_ `errno`-setting function (and 
>> there's _many_ of them).
>> 
>> 
>> ## Proposed Approach
>> 
>> Let's discuss what is a good solution and then I will volunteer to put 
>> together a full proposal.
>> 
>> 
>> ## Source compatibility
>> 
>> it depends.
>> 
>> To retain source compatibility the Darwin/Glibc modules could also be left 
>> as is. The safe `errno` handling could then be implemented only in a new, 
>> unified module for Darwin/Glibc. There's already ongoing 
>> discussions/proposals about that on the list anyway. That new module could 
>> then be implemented in the spirit of options 1, 2, or some other solution. 
>> The benefits are guaranteed source compatibility for legacy applications and 
>> `errno` safety plus easier imports for new applications - win/win 🙂.
>> 
>> 
>> ## Effect on ABI stability
>> 
>> Will most likely be additive, so probably none.
>> 
>> 
>> ## Effect on API resilience
>> 
>> see source compatibility.
>> 
>> 
>> ## Alternatives considered
>> 
>> Do nothing and workaround `errno` capturing being very hard. I discussed 
>> this previously elsewhere and Joe Groff came up with the code below which 
>> should convince the optimiser not to insert any release calls at the wrong 
>> place or inline the function:
>> 
>> ```
>> @inline(never)
>> func callWithErrno(_ fn: () -> Int) -> (result: Int, errno: Int) {
>>  var result: Int
>>  var savedErrno: Int
>>  withExtendedLifetime(fn) {
>>      result = fn()
>>      savedErrno = errno
>>  }
>>  return (result, savedErrno)
>> }
>> ```
>> 
>> An example use of that is
>> 
>> ```
>> let (rv, writeErrno) = callWithErrno {
>>  write(-1, nil, 0)
>> }
>> 
>> if rv < 0 {
>>  throw SomeError(errorCode: writeErrno)
>> }
>> ```
>> 
>> This makes it possible to retrieve the correct `errno` value in Swift but I 
>> think there remain too many ways to do it wrongly. First and foremost that 
>> the compiler doesn't complain if the programmer forgets to use 
>> `callWithErrno`.
>> 
>> ===
>> 
>> Let me know what you think!
>> 
>> [1]: https://en.wikipedia.org/wiki/Errno.h
>> [2]: https://en.wikipedia.org/wiki/System_call
>> [3]: https://golang.org/pkg/syscall/#Write
>> 
>> Many thanks,
>> Johannes
>> _______________________________________________
>> swift-evolution mailing list
>> [email protected]
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
> _______________________________________________
> swift-evolution mailing list
> [email protected]
> https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to