Skip to content

Commit eed9992

Browse files
Keep the current parser in TLS instead of TaskLocal
TaskLocal storage is inherited by non-detached child tasks, which can lead to the parser being shared between tasks. This is not our intention and can lead to inconsistent state. Instead, we should keep the current parser in thread-local storage. This should be safe as long as we don't have any structured suspension points in `withCurrentParser` block.
1 parent 659115f commit eed9992

File tree

1 file changed

+44
-21
lines changed

1 file changed

+44
-21
lines changed

Sources/FoundationXML/XMLParser.swift

+44-21
Original file line numberDiff line numberDiff line change
@@ -462,34 +462,57 @@ open class XMLParser : NSObject {
462462

463463
open var allowedExternalEntityURLs: Set<URL>?
464464

465-
/// The current parser is stored in a task local variable to allow for
466-
/// concurrent parsing in different tasks with different parsers.
467-
///
468-
/// Rationale for `@unchecked Sendable`:
469-
/// While the ``XMLParser`` class itself is not `Sendable`, `TaskLocal`
470-
/// requires the value type to be `Sendable`. The sendability requirement
471-
/// of `TaskLocal` is only for the "default" value and values set with
472-
/// `withValue` will not be shared between tasks.
473-
/// So as long as 1. the default value is safe to be shared between tasks
474-
/// and 2. the `Sendable` conformance of `_CurrentParser` is not used
475-
/// outside of `TaskLocal`, it is safe to mark it as `@unchecked Sendable`.
476-
private struct _CurrentParser: @unchecked Sendable {
477-
let parser: XMLParser?
478-
479-
static var `default`: _CurrentParser {
480-
return _CurrentParser(parser: nil)
465+
/// The current parser context for the current thread.
466+
private class _CurrentParserContext {
467+
var _stack: [XMLParser] = []
468+
var _current: XMLParser? {
469+
return _stack.last
481470
}
482471
}
483472

484-
@TaskLocal
485-
private static var _currentParser: _CurrentParser = .default
473+
#if os(WASI)
474+
/// The current parser associated with the current thread. (assuming no multi-threading)
475+
/// FIXME: Unify the implementation with the other platforms once we unlock `threadDictionary`
476+
/// or migrate to `FoundationEssentials._ThreadLocal`.
477+
private static nonisolated(unsafe) var _currentParserContext: _CurrentParserContext?
478+
#else
479+
/// The current parser associated with the current thread.
480+
private static var _currentParserContext: _CurrentParserContext? {
481+
get {
482+
return Thread.current.threadDictionary["__CurrentNSXMLParser"] as? _CurrentParserContext
483+
}
484+
set {
485+
Thread.current.threadDictionary["__CurrentNSXMLParser"] = newValue
486+
}
487+
}
488+
#endif
486489

490+
/// The current parser associated with the current thread.
487491
internal static func currentParser() -> XMLParser? {
488-
return _currentParser.parser
492+
if let ctx = _currentParserContext {
493+
return ctx._current
494+
}
495+
return nil
489496
}
490-
497+
498+
/// Execute the given closure with the current parser set to the given parser.
491499
internal static func withCurrentParser<R>(_ parser: XMLParser, _ body: () -> R) -> R {
492-
return self.$_currentParser.withValue(_CurrentParser(parser: parser), operation: body)
500+
var ctx: _CurrentParserContext
501+
if let current = _currentParserContext {
502+
// Use the existing context if it exists
503+
ctx = current
504+
} else {
505+
// Create a new context in TLS
506+
ctx = _CurrentParserContext()
507+
_currentParserContext = ctx
508+
}
509+
// Push the parser onto the stack
510+
ctx._stack.append(parser)
511+
defer {
512+
// Pop the parser off the stack
513+
ctx._stack.removeLast()
514+
}
515+
return body()
493516
}
494517

495518
internal func _handleParseResult(_ parseResult: Int32) -> Bool {

0 commit comments

Comments
 (0)