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

SyntaxArena: thread safety #807

Merged
merged 2 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 4 additions & 2 deletions Sources/SwiftParser/Parser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ extension Parser {
// Extended lifetime is required because `SyntaxArena` in the parser must
// be alive until `Syntax(raw:)` retains the arena.
return withExtendedLifetime(parser) {
let rawSourceFile = parser.parseSourceFile()
return Syntax(raw: rawSourceFile.raw).as(SourceFileSyntax.self)!
parser.arena.assumingSingleThread {
let rawSourceFile = parser.parseSourceFile()
return Syntax(raw: rawSourceFile.raw).as(SourceFileSyntax.self)!
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion Sources/SwiftParser/Syntax+StringInterpolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ extension SyntaxExpressibleByStringInterpolation {
var parser = Parser(buffer)
// FIXME: When the parser supports incremental parsing, put the
// interpolatedSyntaxNodes in so we don't have to parse them again.
return Self.parse(from: &parser)
return parser.arena.assumingSingleThread {
return Self.parse(from: &parser)
}
}
}

Expand Down
112 changes: 97 additions & 15 deletions Sources/SwiftSyntax/SyntaxArena.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,60 @@
//
//===----------------------------------------------------------------------===//

#if canImport(Darwin)
import Darwin

class ScopeGuard {
private var lock: os_unfair_lock
init() {
self.lock = os_unfair_lock()
}
func withGuard<T>(body: () throws -> T) rethrows -> T {
os_unfair_lock_lock(&lock)
defer { os_unfair_lock_unlock(&lock)}
return try body()
}
}

#elseif canImport(Glibc)
import Glibc

class ScopeGuard {
var lock: pthread_mutex_t
init() {
self.lock = pthread_mutex_t()
pthread_mutex_init(&self.lock, nil)
}
deinit {
pthread_mutex_destroy(&self.lock)
}
func withGuard<T>(body: () throws -> T) rethrows -> T {
pthread_mutex_lock(&lock)
defer { pthread_mutex_unlock(&lock) }
return try body()
}
}
#else
// FIXME: Support other platforms.

/// Dummy mutex that doesn't actually guard at all.
class ScopeGuard {
init() {}
func withGuard<T>(body: () throws -> T) rethrows -> T {
return try body()
}
}
#endif

public class SyntaxArena {
CodaFi marked this conversation as resolved.
Show resolved Hide resolved

@_spi(RawSyntax)
public typealias ParseTriviaFunction = (_ source: SyntaxText, _ position: TriviaPosition) -> [RawTriviaPiece]

/// Thread safe guard.
private let lock: ScopeGuard
private var singleThreadMode: Bool

/// Bump-pointer allocator for all "intern" methods.
private let allocator: BumpPtrAllocator
/// Source file buffer the Syntax tree represents.
Expand All @@ -30,6 +79,8 @@ public class SyntaxArena {

@_spi(RawSyntax)
public init(parseTriviaFunction: @escaping ParseTriviaFunction) {
lock = ScopeGuard()
self.singleThreadMode = false
allocator = BumpPtrAllocator()
children = []
sourceBuffer = .init(start: nil, count: 0)
Expand All @@ -41,15 +92,32 @@ public class SyntaxArena {
self.init(parseTriviaFunction: _defaultParseTriviaFunction(_:_:))
}

private func withGuard<R>(body: () throws -> R) rethrows -> R {
if self.singleThreadMode {
return try body()
} else {
return try self.lock.withGuard(body: body)
}
}

public func assumingSingleThread<R>(body: () throws -> R) rethrows -> R {
let oldValue = self.singleThreadMode
defer { self.singleThreadMode = oldValue }
self.singleThreadMode = true
return try body()
}

/// Copies a source buffer in to the memory this arena manages, and returns
/// the interned buffer.
///
/// The interned buffer is guaranteed to be null-terminated.
/// `contains(address _:)` is faster if the address is inside the memory
/// range this function returned.
public func internSourceBuffer(_ buffer: UnsafeBufferPointer<UInt8>) -> UnsafeBufferPointer<UInt8> {
let allocated = lock.withGuard {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be self.withGuard?

allocator.allocate(UInt8.self, count: buffer.count + /* for NULL */1)
}
precondition(sourceBuffer.baseAddress == nil, "SourceBuffer should only be set once.")
let allocated = allocator.allocate(UInt8.self, count: buffer.count + /* for NULL */1)
_ = allocated.initialize(from: buffer)

// NULL terminate.
Expand All @@ -69,20 +137,27 @@ public class SyntaxArena {
/// Allocates a buffer of `RawSyntax?` with the given count, then returns the
/// uninitlialized memory range as a `UnsafeMutableBufferPointer<RawSyntax?>`.
func allocateRawSyntaxBuffer(count: Int) -> UnsafeMutableBufferPointer<RawSyntax?> {
return allocator.allocate(RawSyntax?.self, count: count)
return self.withGuard {
allocator.allocate(RawSyntax?.self, count: count)
}
}

/// Allcates a buffer of `RawTriviaPiece` with the given count, then returns
/// the uninitialized memory range as a `UnsafeMutableBufferPointer<RawTriviaPiece>`.
func allocateRawTriviaPieceBuffer(
count: Int) -> UnsafeMutableBufferPointer<RawTriviaPiece> {
return allocator.allocate(RawTriviaPiece.self, count: count)
count: Int
) -> UnsafeMutableBufferPointer<RawTriviaPiece> {
return self.withGuard {
allocator.allocate(RawTriviaPiece.self, count: count)
}
}

/// Allcates a buffer of `UInt8` with the given count, then returns the
/// uninitialized memory range as a `UnsafeMutableBufferPointer<UInt8>`.
func allocateTextBuffer(count: Int) -> UnsafeMutableBufferPointer<UInt8> {
return allocator.allocate(UInt8.self, count: count)
return self.withGuard {
allocator.allocate(UInt8.self, count: count)
}
}

/// Copies the contents of a `SyntaxText` to the memory this arena manages,
Expand Down Expand Up @@ -114,7 +189,9 @@ public class SyntaxArena {
/// Copies a `RawSyntaxData` to the memory this arena manages, and retuns the
/// pointer to the destination.
func intern(_ value: RawSyntaxData) -> UnsafePointer<RawSyntaxData> {
let allocated = allocator.allocate(RawSyntaxData.self, count: 1).baseAddress!
let allocated = lock.withGuard {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, should this be self.withGuard?

allocator.allocate(RawSyntaxData.self, count: 1).baseAddress!
}
allocated.initialize(to: value)
return UnsafePointer(allocated)
}
Expand All @@ -128,21 +205,26 @@ public class SyntaxArena {
/// See also `RawSyntax.layout()`.
func addChild(_ arenaRef: SyntaxArenaRef) {
if SyntaxArenaRef(self) == arenaRef { return }

let other = arenaRef.value

precondition(
!self.hasParent,
"an arena can't have a new child once it's owned by other arenas")

other.hasParent = true
children.insert(other)
other.withGuard {
self.withGuard {
precondition(
!self.hasParent,
"an arena can't have a new child once it's owned by other arenas")

other.hasParent = true
children.insert(other)
}
}
}

/// Recursively checks if this arena contains given `arena` as a descendant.
func contains(arena: SyntaxArena) -> Bool {
return children.contains { child in
child === arena || child.contains(arena: arena)
self.withGuard {
children.contains { child in
child === arena || child.contains(arena: arena)
}
}
}

Expand Down
21 changes: 20 additions & 1 deletion Tests/SwiftSyntaxTest/MultithreadingTests.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import XCTest
import SwiftSyntax
@_spi(RawSyntax) import SwiftSyntax


public class MultithreadingTests: XCTestCase {

Expand All @@ -15,6 +16,24 @@ public class MultithreadingTests: XCTestCase {
}
}

public func testConcurrentArena() {
let arena = SyntaxArena()

DispatchQueue.concurrentPerform(iterations: 100) { i in
var identStr = " ident\(i) "
let tokenRaw = identStr.withSyntaxText { text in
RawTokenSyntax(
kind: .identifier,
wholeText: arena.intern(text),
textRange: 1..<(text.count-1),
presence: .present,
arena: arena)
}
let token = Syntax(raw: RawSyntax(tokenRaw)).as(TokenSyntax.self)!
XCTAssertEqual(token.text, "ident\(i)")
}
}

public func testTwoAccesses() {
let tuple = TupleTypeSyntax(
leftParen: .leftParenToken(),
Expand Down