Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix EventLoopFuture and EventLoopPromise under strict concurrency checking #2654

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Commits on Feb 19, 2024

  1. Fix EventLoopFuture and EventLoopPromise under strict concurrency…

    … checking
    
    # Motivation
    
    We need to tackle the remaining strict concurrency checking related `Sendable` warnings in NIO. The first place to start is making sure that `EventLoopFuture` and `EventLoopPromise` are properly annotated.
    
    # Modification
    
    In a previous apple#2496, @weissi changed the `@unchecked Sendable` conformances of `EventLoopFuture/Promise` to be conditional on the sendability of the generic `Value` type. After having looked at all the APIs on the future and promise types as well as reading the latest Concurrency evolution proposals, specifically the [Region based Isolation](https://github.com/apple/swift-evolution/blob/main/proposals/0414-region-based-isolation.md), I came to the conclusion that the previous `@unchecked Sendable` annotations were correct. The reasoning for this is:
    
    1. An `EventLoopPromise` and `EventLoopFuture` pair are tied to a specific `EventLoop`
    2. An `EventLoop` represents an isolation region and values tied to its isolation are not allowed to be shared outside of it unless they are disconnected from the region
    3. The `value` used to succeed a promise often come from outside the isolation domain of the `EventLoop` hence they must be transferred into the promise.
    4. The isolation region of the event loop is enforced through `@Sendable` annotations on all closures that receive the value in some kind of transformation e.g. `map()`
    5. Any method on `EventLoopFuture` that combines itself with another future must require `Sendable` of the other futures `Value` since we cannot statically enforce that futures are bound to the same event loop i.e. to the same isolation domain
    
    Due to the above rules, this PR adds back the `@unchecked Sendable` conformances to both types. Furthermore, this PR revisits every single method on `EventLoopPromise/Future` and adds missing `Sendable` and `@Sendable` annotation where necessary to uphold the above rules. A few important things to call out:
    
    - Since `transferring` is currently not available this PR requires a `Sendable` conformance for some methods on `EventLoopPromise/Future` that should rather take a `transffering` argument
    - To enable the common case where a value from the same event loop is used to succeed a promise I added two additional methods that take a `eventLoopBoundResult` and enforce dynamic isolation checking. We might have to do this for more methods once we adopt those changes in other targets/packages.
    
    # Result
    
    After this PR has landed our lowest level building block should be inline with what the rest of the language enforces in Concurrency. The `EventLoopFuture.swift` produces no more warnings under strict concurrency checking on the latest 5.10 snapshots.
    FranzBusch committed Feb 19, 2024
    Configuration menu
    Copy the full SHA
    e0a87fc View commit details
    Browse the repository at this point in the history
  2. George review

    FranzBusch committed Feb 19, 2024
    Configuration menu
    Copy the full SHA
    5d20621 View commit details
    Browse the repository at this point in the history

Commits on Mar 4, 2024

  1. Configuration menu
    Copy the full SHA
    3ae701c View commit details
    Browse the repository at this point in the history

Commits on Oct 9, 2024

  1. Configuration menu
    Copy the full SHA
    aeb2634 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    469e475 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    426e453 View commit details
    Browse the repository at this point in the history

Commits on Oct 16, 2024

  1. Configuration menu
    Copy the full SHA
    7f83110 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    b191621 View commit details
    Browse the repository at this point in the history
  3. Formatting

    Lukasa committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    b9b9ea6 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    77da82c View commit details
    Browse the repository at this point in the history

Commits on Oct 17, 2024

  1. Configuration menu
    Copy the full SHA
    bfe84d2 View commit details
    Browse the repository at this point in the history
  2. Further tweaks

    Lukasa committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    75ad4d7 View commit details
    Browse the repository at this point in the history
  3. Add explanatory documentation

    Lukasa committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    28029bf View commit details
    Browse the repository at this point in the history
  4. Formatting

    Lukasa committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    dacc452 View commit details
    Browse the repository at this point in the history
  5. More formatting

    Lukasa committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    c41d45d View commit details
    Browse the repository at this point in the history