Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ let defaultSwiftSettings: [SwiftSetting] =
.enableUpcomingFeature("ExistentialAny"),
.enableUpcomingFeature("InternalImportsByDefault"),
.enableUpcomingFeature("MemberImportVisibility"),
.unsafeFlags(["-Xfrontend", "-require-explicit-sendable"]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't unconditionally add this flag here; these flags are always enabled. This one should only be added for development.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to remove it! Thanks for the reminder.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can still have it so we can enable it during development though - here's an example: https://github.com/apple/swift-nio-ssl/blob/main/Package.swift#L59-L73

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let me add this then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to add it back behind a flag. It was helpful when we were doing lots of strict concurrency work, but we're very unlikely to flip that flag back on now that the work has been done.

]

// -------------------------------------------------------------------------------------------------
Expand Down
21 changes: 12 additions & 9 deletions Sources/GRPCCodeGen/CodeGenerationRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ public struct CodeGenerationRequest {
}
}

@available(*, unavailable)
extension CodeGenerationRequest: Sendable {}

@available(gRPCSwift 2.0, *)
extension CodeGenerationRequest {
@available(*, deprecated, renamed: "makeSerializerSnippet")
Expand Down Expand Up @@ -123,7 +126,7 @@ extension CodeGenerationRequest {

/// Represents an import: a module or a specific item from a module.
@available(gRPCSwift 2.0, *)
public struct Dependency: Equatable {
public struct Dependency: Equatable, Sendable {
/// If the dependency is an item, the property's value is the item representation.
/// If the dependency is a module, this property is nil.
public var item: Item?
Expand Down Expand Up @@ -158,7 +161,7 @@ public struct Dependency: Equatable {
}

/// Represents an item imported from a module.
public struct Item: Equatable {
public struct Item: Equatable, Sendable {
/// The keyword that specifies the item's kind (e.g. `func`, `struct`).
public var kind: Kind

Expand All @@ -171,7 +174,7 @@ public struct Dependency: Equatable {
}

/// Represents the imported item's kind.
public struct Kind: Equatable {
public struct Kind: Equatable, Sendable {
/// Describes the keyword associated with the imported item.
internal enum Value: String {
case `typealias`
Expand Down Expand Up @@ -233,7 +236,7 @@ public struct Dependency: Equatable {
}

/// Describes any requirement for the `@preconcurrency` attribute.
public struct PreconcurrencyRequirement: Equatable {
public struct PreconcurrencyRequirement: Equatable, Sendable {
internal enum Value: Equatable {
case required
case notRequired
Expand Down Expand Up @@ -265,7 +268,7 @@ public struct Dependency: Equatable {

/// Represents a service described in an IDL file.
@available(gRPCSwift 2.0, *)
public struct ServiceDescriptor: Hashable {
public struct ServiceDescriptor: Hashable, Sendable {
/// Documentation from comments above the IDL service description.
/// It is already formatted, meaning it contains "///" and new lines.
public var documentation: String
Expand Down Expand Up @@ -323,7 +326,7 @@ extension ServiceDescriptor {

/// Represents a method described in an IDL file.
@available(gRPCSwift 2.0, *)
public struct MethodDescriptor: Hashable {
public struct MethodDescriptor: Hashable, Sendable {
/// Documentation from comments above the IDL method description.
/// It is already formatted, meaning it contains "///" and new lines.
public var documentation: String
Expand Down Expand Up @@ -388,7 +391,7 @@ extension MethodDescriptor {
}

@available(gRPCSwift 2.0, *)
public struct ServiceName: Hashable {
public struct ServiceName: Hashable, Sendable {
/// The identifying name as used in the service/method descriptors including any namespace.
///
/// This value is also used to identify the service to the remote peer, usually as part of the
Expand Down Expand Up @@ -423,7 +426,7 @@ public struct ServiceName: Hashable {
}

@available(gRPCSwift 2.0, *)
public struct MethodName: Hashable {
public struct MethodName: Hashable, Sendable {
/// The identifying name as used in the service/method descriptors.
///
/// This value is also used to identify the method to the remote peer, usually as part of the
Expand Down Expand Up @@ -455,7 +458,7 @@ public struct MethodName: Hashable {
/// Represents the name associated with a namespace, service or a method, in three different formats.
@available(*, deprecated, message: "Use ServiceName/MethodName instead.")
@available(gRPCSwift 2.0, *)
public struct Name: Hashable {
public struct Name: Hashable, Sendable {
/// The base name is the name used for the namespace/service/method in the IDL file, so it should follow
/// the specific casing of the IDL.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ extension ClientRPCExecutor.HedgingExecutor {
}

@usableFromInline
struct ScheduledState {
struct ScheduledState: Sendable {
@usableFromInline
var _handle: CancellableTaskHandle?
@usableFromInline
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

@available(gRPCSwift 2.0, *)
@usableFromInline
enum ClientRPCExecutor {
enum ClientRPCExecutor: Sendable {
/// Execute the request and handle its response.
///
/// - Parameters:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

@available(gRPCSwift 2.0, *)
@usableFromInline
internal enum ClientStreamExecutor {
internal enum ClientStreamExecutor: Sendable {
/// Execute a request on the stream executor.
///
/// - Parameters:
Expand Down Expand Up @@ -250,3 +250,6 @@ internal enum ClientStreamExecutor {
}
}
}

@available(*, unavailable)
extension ClientStreamExecutor.RawBodyPartToMessageSequence.AsyncIterator: Sendable {}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public import Musl // should be @usableFromInline

@available(gRPCSwift 2.0, *)
@usableFromInline
struct RetryDelaySequence: Sequence {
struct RetryDelaySequence: Sequence, Sendable {
@usableFromInline
typealias Element = Duration

Expand All @@ -45,7 +45,7 @@ struct RetryDelaySequence: Sequence {
}

@usableFromInline
struct Iterator: IteratorProtocol {
struct Iterator: IteratorProtocol, Sendable {
@usableFromInline
let policy: RetryPolicy
@usableFromInline
Expand Down
4 changes: 2 additions & 2 deletions Sources/GRPCCore/Call/Server/Internal/ServerRPCExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

@available(gRPCSwift 2.0, *)
@usableFromInline
struct ServerRPCExecutor {
struct ServerRPCExecutor: Sendable {
/// Executes an RPC using the provided handler.
///
/// - Parameters:
Expand Down Expand Up @@ -272,7 +272,7 @@ struct ServerRPCExecutor {
}

@usableFromInline
enum OnFirstRequestPart<Bytes: GRPCContiguousBytes> {
enum OnFirstRequestPart<Bytes: GRPCContiguousBytes>: Sendable {
case process(
Metadata,
UnsafeTransfer<RPCAsyncSequence<RPCRequestPart<Bytes>, any Error>.AsyncIterator>
Expand Down
4 changes: 2 additions & 2 deletions Sources/GRPCCore/Coding/CompressionAlgorithm.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ extension CompressionAlgorithmSet {
}

/// A sequence of ``CompressionAlgorithm`` values present in a ``CompressionAlgorithmSet``.
public struct Elements: Sequence {
public struct Elements: Sequence, Sendable {
public typealias Element = CompressionAlgorithm

private let algorithmSet: CompressionAlgorithmSet
Expand All @@ -107,7 +107,7 @@ extension CompressionAlgorithmSet {
return Iterator(algorithmSet: self.algorithmSet)
}

public struct Iterator: IteratorProtocol {
public struct Iterator: IteratorProtocol, Sendable {
private let algorithmSet: CompressionAlgorithmSet
private var iterator: IndexingIterator<[CompressionAlgorithm.Value]>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ struct AsyncSequenceOfOne<Element: Sendable, Failure: Error>: AsyncSequence, Sen
}

@usableFromInline
struct AsyncIterator: AsyncIteratorProtocol {
struct AsyncIterator: AsyncIteratorProtocol, Sendable {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, iterators aren't Sendable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I mark it as not Sendable even though it currently conforms to Sendable then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact it implicitly conforms to Sendable right now because its implementation ticks all the boxes doesn't mean it should be Sendable when you consider the semantics. In general, all iterators keep some internal state to know what the next item should be. If an iterator is shared between different tasks, then it becomes undeterministic which task gets which element, which isn't good.

Also, I left the comment on this iterator but this should be true for all other iterators marked as Sendable on this PR.

@usableFromInline
private(set) var result: Result<Element, Failure>?

Expand Down
40 changes: 35 additions & 5 deletions Sources/GRPCCore/Streaming/Internal/BroadcastAsyncSequence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ struct BroadcastAsyncSequence<Element: Sendable>: Sendable, AsyncSequence {
@available(gRPCSwift 2.0, *)
extension BroadcastAsyncSequence {
@usableFromInline
struct AsyncIterator: AsyncIteratorProtocol {
struct AsyncIterator: AsyncIteratorProtocol, Sendable {
@usableFromInline
let _storage: _BroadcastSequenceStorage<Element>
@usableFromInline
Expand Down Expand Up @@ -1204,7 +1204,7 @@ struct _BroadcastSequenceStateMachine<Element: Sendable>: Sendable {
}

@usableFromInline
enum OnCancelSubscription {
enum OnCancelSubscription: Sendable {
case none
case resume(ConsumerContinuation, Result<Element?, any Error>)
}
Expand Down Expand Up @@ -1244,7 +1244,7 @@ struct _BroadcastSequenceStateMachine<Element: Sendable>: Sendable {
}

@usableFromInline
enum OnSubscribe {
enum OnSubscribe: Sendable {
case subscribed(_BroadcastSequenceStateMachine<Element>.Subscriptions.ID)
}

Expand Down Expand Up @@ -1282,7 +1282,7 @@ struct _BroadcastSequenceStateMachine<Element: Sendable>: Sendable {
}

@usableFromInline
enum OnWaitToProduceMore {
enum OnWaitToProduceMore: Sendable {
case none
case resume(ProducerContinuation, Result<Void, any Error>)
}
Expand Down Expand Up @@ -1498,7 +1498,7 @@ extension _BroadcastSequenceStateMachine {
}

@usableFromInline
enum ElementLookup {
enum ElementLookup: Sendable {
/// The element was found in the collection.
case found(Element)
/// The element isn't in the collection, but it could be in the future.
Expand Down Expand Up @@ -1810,3 +1810,33 @@ extension _BroadcastSequenceStateMachine {
case many([Value])
}
}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.ConsumerContinuations: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.ProducerContinuations: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnInvalidateAllSubscriptions: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnYield: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnFinish: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnNext: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnNext.ReturnAndResumeProducers: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnSetContinuation: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine.OnDropResources: Sendable {}

@available(*, unavailable)
extension _BroadcastSequenceStateMachine._OneOrMany: Sendable {}
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,6 @@ final class UncheckedAsyncIteratorSequence<
return AsyncIterator(base: self.base)
}
}

@available(*, unavailable)
extension UncheckedAsyncIteratorSequence.AsyncIterator: Sendable {}
Loading