Improve @SendableProperty (#91)

This PR ensures that we enter a lock inside the `@SendableProperty`
implementation as soon as we access a computed property. Additionally,
it mirrors the access level of the original property. Both changes are
required for [improved
accuracy](https://github.com/apple/container/pull/144) of progress
updates in container. Additionally, it should resolve
https://github.com/apple/containerization/issues/60 that occurs on
certain configurations.

Please tag as 0.1.1 after merging.
This commit is contained in:
Dmitry Kovba
2025-06-11 14:13:21 -04:00
committed by GitHub
parent 0cb8d29462
commit f245ae63b3
6 changed files with 39 additions and 45 deletions
@@ -22,6 +22,7 @@ import ContainerizationOCI
import Foundation
import Logging
import SendableProperty
import Synchronization
import struct ContainerizationOS.Terminal
@@ -177,10 +177,10 @@ extension RegistryClient {
while var buf = try await itr.next() {
let readBytes = Int64(buf.readableBytes)
received += readBytes
await progress?([
ProgressEvent(event: "add-size", value: readBytes)
])
let written = try await writer.write(contentsOf: buf)
await progress?([
ProgressEvent(event: "add-size", value: written)
])
guard written == readBytes else {
throw ContainerizationError(.internalError, message: "Could not write \(readBytes) bytes to file \(file)")
}
@@ -15,10 +15,32 @@
// limitations under the License.
//===----------------------------------------------------------------------===//
// `Synchronization` will be automatically imported with `SendableProperty`
@_exported import Synchronization
// `Foundation` will be automatically imported with `SendableProperty`.
@_exported import Foundation
// A declaration of the `@SendableProperty` macro.
@attached(peer, names: arbitrary)
@attached(accessor)
public macro SendableProperty() = #externalMacro(module: "SendablePropertyMacros", type: "SendablePropertyMacro")
/// A synchronization primitive that protects shared mutable state via mutual exclusion.
public final class Synchronized<T>: @unchecked Sendable {
private let lock = NSLock()
private var value: T
/// Creates a new instance.
/// - Parameter value: The initial value.
public init(_ value: T) {
self.value = value
}
/// Calls the given closure after acquiring the lock and returns its value.
/// - Parameter body: The body of code to execute while the lock is held.
public func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R {
lock.lock()
defer {
lock.unlock()
}
return try body(&value)
}
}
@@ -21,7 +21,6 @@ import SwiftParser
import SwiftSyntax
import SwiftSyntaxBuilder
import SwiftSyntaxMacros
import Synchronization
/// A macro that allows to make a property thread-safe keeping the `Sendable` conformance of the type.
public struct SendablePropertyMacro: PeerMacro {
@@ -54,12 +53,13 @@ public struct SendablePropertyMacro: PeerMacro {
genericTypeAnnotation = "<\(typeName)\(hasInitializer ? "" : "?")>"
}
let accessLevel = varDecl.modifiers.first(where: { ["open", "public", "internal", "fileprivate", "private"].contains($0.name.text) })?.name.text ?? "internal"
// Create a peer property
let peerPropertyName = self.peerPropertyName(for: propertyName)
// `Mutex` (requires macOS 15) and `OSAllocationUnfairLock` (requires macOS 13, unsupported on Linux) are more effective than `NSLock`.
let peerProperty: DeclSyntax =
"""
private let \(raw: peerPropertyName) = Mutex\(raw: genericTypeAnnotation)(\(raw: initializerValue))
\(raw: accessLevel) let \(raw: peerPropertyName) = Synchronized\(raw: genericTypeAnnotation)(\(raw: initializerValue))
"""
return [peerProperty]
}
@@ -93,18 +93,10 @@ extension SendablePropertyMacro: AccessorMacro {
\(raw: peerPropertyName).withLock { $0\(raw: hasInitializer ? "" : "!") }
}
"""
// The `Sending` class is used as a temporary workaround for the error: "'inout sending' parameter '$0' cannot be task-isolated at end of function."
let accessorSetter: AccessorDeclSyntax =
"""
set {
class Sending<T>: @unchecked Sendable {
let wrappedValue: T
init(_ value: T) {
wrappedValue = value
}
}
let newValue = Sending(newValue)
\(raw: peerPropertyName).withLock { $0 = newValue.wrappedValue }
\(raw: peerPropertyName).withLock { $0 = newValue }
}
"""
@@ -62,7 +62,7 @@ public class ImageStoreTests: ContainsAuth {
return
}
let imageReference = "ghcr.io/apple/containerization/dockermanifestimage:0.0.2"
let busyboxImage = try await self.store.pull(reference: imageReference, auth: Self.authentication)
let busyboxImage = try await self.store.pull(reference: imageReference, auth: authentication)
let got = try await self.store.get(reference: imageReference)
#expect(got.descriptor == busyboxImage.descriptor)
@@ -52,20 +52,13 @@ final class SendablePropertyMacrosTests: XCTestCase {
}
}
set {
class Sending<T>: @unchecked Sendable {
let wrappedValue: T
init(_ value: T) {
wrappedValue = value
}
}
let newValue = Sending(newValue)
_value.withLock {
$0 = newValue.wrappedValue
$0 = newValue
}
}
}
private let _value = Mutex<Int?>(nil)
internal let _value = Synchronized<Int?>(nil)
}
""",
macros: testMacros
@@ -94,20 +87,13 @@ final class SendablePropertyMacrosTests: XCTestCase {
}
}
set {
class Sending<T>: @unchecked Sendable {
let wrappedValue: T
init(_ value: T) {
wrappedValue = value
}
}
let newValue = Sending(newValue)
_value.withLock {
$0 = newValue.wrappedValue
$0 = newValue
}
}
}
private let _value = Mutex(0)
internal let _value = Synchronized(0)
}
""",
macros: testMacros
@@ -136,20 +122,13 @@ final class SendablePropertyMacrosTests: XCTestCase {
}
}
set {
class Sending<T>: @unchecked Sendable {
let wrappedValue: T
init(_ value: T) {
wrappedValue = value
}
}
let newValue = Sending(newValue)
_value.withLock {
$0 = newValue.wrappedValue
$0 = newValue
}
}
}
private let _value = Mutex<Int>(0)
internal let _value = Synchronized<Int>(0)
}
""",
macros: testMacros