Skip to content

Conversation

@KeithBauerANZ
Copy link
Collaborator

📒 Description

FlagValueSourceCoordinator initializer wasn't safe if the non-sendable source continued to be used after it was secured by the lock.

🔍 Detailed Design

This PR splits the FlagValueSourceCoordinator initializer in two:

The first makes it clear (unchecked) that the operation is unsafe. I've chosen "unchecked" by analogy with OSAllocatedUnfairLock.

    /// Create a FlagValueSource from a NonSendableFlagValueSource. If `Source` is a reference type,
    /// you must not continue to access it (except via this coordinator) after passing it to this
    /// initializer.
    public init(uncheckedSource source: Source) { ... }

The second, available in Swift 6 and later, makes the operation safe by guaranteeing that the non-Sendable value can't be shared outside the lock:

#if swift(>=6)
    /// Create a FlagValueSource from a NonSendableFlagValueSource.
    public init(source: sending Source) { ... }
#endif

📓 Documentation Plan

This type is not yet documented outside doc comments. I haven't added any, though.

🗳 Test Plan

FVSC is not currently tested. I haven't added any, though.

🧯 Source Impact

FVSC is new to Vexil 3, so this isn't [more] breaking to users of 2.x

✅ Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

@KeithBauerANZ KeithBauerANZ requested a review from bok- August 2, 2024 13:17
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2024

@KeithBauerANZ KeithBauerANZ added the vexil3 Part of the Vexil 3 alpha/beta development label Aug 2, 2024
@bok- bok- enabled auto-merge December 10, 2024 12:47
@sonarqubecloud
Copy link

@bok- bok- merged commit f0c4867 into main Dec 10, 2024
26 of 34 checks passed
@bok- bok- deleted the keith/fvsc-safety branch December 10, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vexil3 Part of the Vexil 3 alpha/beta development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants