1
- From 9df174b737801ba7787ab59c5f34c3dc53b755ec Mon Sep 17 00:00:00 2001
1
+ From 6a78203fc24702ed81a2b2bb2a9949168e694161 Mon Sep 17 00:00:00 2001
2
2
From: Yuta Saito <
[email protected] >
3
3
Date: Thu, 8 Aug 2024 23:33:15 +0000
4
- Subject: [PATCH] [XMLParser] Use `TaskLocal` for storing the current parser
4
+ Subject: [PATCH 1/3] [XMLParser] Use `TaskLocal` for storing the current
5
+ parser
5
6
6
7
Instead of thread-local storage, use `TaskLocal` to store the current
7
8
parser. This solves three issues:
@@ -26,11 +27,11 @@ parser. This solves three issues:
26
27
platform-specific assumption as much as possible.
27
28
---
28
29
Sources/FoundationXML/XMLParser.swift | 85 ++++++++++-----------------
29
- Tests/Foundation/TestXMLParser.swift | 40 ++++++++++++-
30
- 2 files changed, 71 insertions(+), 54 deletions(-)
30
+ Tests/Foundation/TestXMLParser.swift | 43 + ++++++++++++-
31
+ 2 files changed, 74 insertions(+), 54 deletions(-)
31
32
32
33
diff --git a/Sources/FoundationXML/XMLParser.swift b/Sources/FoundationXML/XMLParser.swift
33
- index d89d0ee1..e3d718a8 100644
34
+ index d89d0ee1f4..e3d718a86f 100644
34
35
--- a/Sources/FoundationXML/XMLParser.swift
35
36
+++ b/Sources/FoundationXML/XMLParser.swift
36
37
@@ -398,9 +398,7 @@ extension XMLParser : @unchecked Sendable { }
@@ -156,10 +157,10 @@ index d89d0ee1..e3d718a8 100644
156
157
157
158
// called by the delegate to stop the parse. The delegate will get an error message sent to it.
158
159
diff --git a/Tests/Foundation/TestXMLParser.swift b/Tests/Foundation/TestXMLParser.swift
159
- index c98741eb..e5e2df4b 100644
160
+ index c98741eb38..df3685a82e 100644
160
161
--- a/Tests/Foundation/TestXMLParser.swift
161
162
+++ b/Tests/Foundation/TestXMLParser.swift
162
- @@ -198,5 +198,43 @@ class TestXMLParser : XCTestCase {
163
+ @@ -198,5 +198,46 @@ class TestXMLParser : XCTestCase {
163
164
ElementNameChecker("noPrefix").check()
164
165
ElementNameChecker("myPrefix:myLocalName").check()
165
166
}
@@ -173,7 +174,7 @@ index c98741eb..e5e2df4b 100644
173
174
+ // ignore any external entity related configuration.
174
175
+ let childParser = XMLParser(data: "<child />".data(using: .utf8)!)
175
176
+ XCTAssertTrue(childParser.parse())
176
- + super.parserDidStartDocument(childParser )
177
+ + super.parserDidStartDocument(parser )
177
178
+ }
178
179
+ }
179
180
+ try withTemporaryDirectory { dir, _ in
@@ -192,8 +193,11 @@ index c98741eb..e5e2df4b 100644
192
193
+ parser.externalEntityResolvingPolicy = .never
193
194
+ let delegate = Delegate()
194
195
+ parser.delegate = delegate
195
- + XCTAssertTrue(parser.parse())
196
- + XCTAssertNil(parser.parserError)
196
+ + // The parse result changes depending on the libxml2 version
197
+ + // because of the following libxml2 commit (shipped in libxml2 2.9.10):
198
+ + // https://gitlab.gnome.org/GNOME/libxml2/-/commit/eddfbc38fa7e84ccd480eab3738e40d1b2c83979
199
+ + // So we don't check the parse result here.
200
+ + _ = parser.parse()
197
201
+ XCTAssertEqual(delegate.events, [
198
202
+ .startDocument,
199
203
+ .didStartElement("doc", nil, nil, [:]),
@@ -204,6 +208,150 @@ index c98741eb..e5e2df4b 100644
204
208
+ }
205
209
+ }
206
210
}
207
- - -
208
- 2.43.2
209
211
212
+ From 7a6125f16e07bb9cdd15e8bee5e7e2c283da4205 Mon Sep 17 00:00:00 2001
213
+ From: Yuta Saito <
[email protected] >
214
+ Date: Fri, 9 Aug 2024 01:51:50 +0000
215
+ Subject: [PATCH 2/3] Remove unnecessary `#if os(WASI)` condition in
216
+ XMLParser.swift
217
+
218
+ ---
219
+ Sources/FoundationXML/XMLParser.swift | 6 ------
220
+ 1 file changed, 6 deletions(-)
221
+
222
+ diff --git a/Sources/FoundationXML/XMLParser.swift b/Sources/FoundationXML/XMLParser.swift
223
+ index e3d718a86f..39eea6c3d8 100644
224
+ --- a/Sources/FoundationXML/XMLParser.swift
225
+ +++ b/Sources/FoundationXML/XMLParser.swift
226
+ @@ -412,9 +412,6 @@ open class XMLParser : NSObject {
227
+
228
+ // initializes the parser with the specified URL.
229
+ public convenience init?(contentsOf url: URL) {
230
+ - #if os(WASI)
231
+ - return nil
232
+ - #else
233
+ setupXMLParsing()
234
+ if url.isFileURL {
235
+ if let stream = InputStream(url: url) {
236
+ @@ -432,7 +429,6 @@ open class XMLParser : NSObject {
237
+ return nil
238
+ }
239
+ }
240
+ - #endif
241
+ }
242
+
243
+ // create the parser from data
244
+ @@ -448,7 +444,6 @@ open class XMLParser : NSObject {
245
+ _CFXMLInterfaceDestroyContext(_parserContext)
246
+ }
247
+
248
+ - #if !os(WASI)
249
+ //create a parser that incrementally pulls data from the specified stream and parses it.
250
+ public init(stream: InputStream) {
251
+ setupXMLParsing()
252
+ @@ -456,7 +451,6 @@ open class XMLParser : NSObject {
253
+ _handler = _CFXMLInterfaceCreateSAXHandler()
254
+ _parserContext = nil
255
+ }
256
+ - #endif
257
+
258
+ open weak var delegate: XMLParserDelegate?
259
+
260
+
261
+ From a4a80e1c2f6721b7e92e137d10dbf37dacb65be9 Mon Sep 17 00:00:00 2001
262
+ From: Yuta Saito <
[email protected] >
263
+ Date: Fri, 23 Aug 2024 06:20:59 +0000
264
+ Subject: [PATCH 3/3] Keep the current parser in TLS instead of TaskLocal
265
+
266
+ TaskLocal storage is inherited by non-detached child tasks, which can
267
+ lead to the parser being shared between tasks. This is not our intention
268
+ and can lead to inconsistent state. Instead, we should keep the current
269
+ parser in thread-local storage. This should be safe as long as we don't
270
+ have any structured suspension points in `withCurrentParser` block.
271
+ ---
272
+ Sources/FoundationXML/XMLParser.swift | 65 ++++++++++++++++++---------
273
+ 1 file changed, 44 insertions(+), 21 deletions(-)
274
+
275
+ diff --git a/Sources/FoundationXML/XMLParser.swift b/Sources/FoundationXML/XMLParser.swift
276
+ index 39eea6c3d8..952c25cd58 100644
277
+ --- a/Sources/FoundationXML/XMLParser.swift
278
+ +++ b/Sources/FoundationXML/XMLParser.swift
279
+ @@ -462,34 +462,57 @@ open class XMLParser : NSObject {
280
+
281
+ open var allowedExternalEntityURLs: Set<URL>?
282
+
283
+ - /// The current parser is stored in a task local variable to allow for
284
+ - /// concurrent parsing in different tasks with different parsers.
285
+ - ///
286
+ - /// Rationale for `@unchecked Sendable`:
287
+ - /// While the ``XMLParser`` class itself is not `Sendable`, `TaskLocal`
288
+ - /// requires the value type to be `Sendable`. The sendability requirement
289
+ - /// of `TaskLocal` is only for the "default" value and values set with
290
+ - /// `withValue` will not be shared between tasks.
291
+ - /// So as long as 1. the default value is safe to be shared between tasks
292
+ - /// and 2. the `Sendable` conformance of `_CurrentParser` is not used
293
+ - /// outside of `TaskLocal`, it is safe to mark it as `@unchecked Sendable`.
294
+ - private struct _CurrentParser: @unchecked Sendable {
295
+ - let parser: XMLParser?
296
+ -
297
+ - static var `default`: _CurrentParser {
298
+ - return _CurrentParser(parser: nil)
299
+ + /// The current parser context for the current thread.
300
+ + private class _CurrentParserContext {
301
+ + var _stack: [XMLParser] = []
302
+ + var _current: XMLParser? {
303
+ + return _stack.last
304
+ }
305
+ }
306
+
307
+ - @TaskLocal
308
+ - private static var _currentParser: _CurrentParser = .default
309
+ + #if os(WASI)
310
+ + /// The current parser associated with the current thread. (assuming no multi-threading)
311
+ + /// FIXME: Unify the implementation with the other platforms once we unlock `threadDictionary`
312
+ + /// or migrate to `FoundationEssentials._ThreadLocal`.
313
+ + private static nonisolated(unsafe) var _currentParserContext: _CurrentParserContext?
314
+ + #else
315
+ + /// The current parser associated with the current thread.
316
+ + private static var _currentParserContext: _CurrentParserContext? {
317
+ + get {
318
+ + return Thread.current.threadDictionary["__CurrentNSXMLParser"] as? _CurrentParserContext
319
+ + }
320
+ + set {
321
+ + Thread.current.threadDictionary["__CurrentNSXMLParser"] = newValue
322
+ + }
323
+ + }
324
+ + #endif
325
+
326
+ + /// The current parser associated with the current thread.
327
+ internal static func currentParser() -> XMLParser? {
328
+ - return _currentParser.parser
329
+ + if let ctx = _currentParserContext {
330
+ + return ctx._current
331
+ + }
332
+ + return nil
333
+ }
334
+ -
335
+ +
336
+ + /// Execute the given closure with the current parser set to the given parser.
337
+ internal static func withCurrentParser<R>(_ parser: XMLParser, _ body: () -> R) -> R {
338
+ - return self.$_currentParser.withValue(_CurrentParser(parser: parser), operation: body)
339
+ + var ctx: _CurrentParserContext
340
+ + if let current = _currentParserContext {
341
+ + // Use the existing context if it exists
342
+ + ctx = current
343
+ + } else {
344
+ + // Create a new context in TLS
345
+ + ctx = _CurrentParserContext()
346
+ + _currentParserContext = ctx
347
+ + }
348
+ + // Push the parser onto the stack
349
+ + ctx._stack.append(parser)
350
+ + defer {
351
+ + // Pop the parser off the stack
352
+ + ctx._stack.removeLast()
353
+ + }
354
+ + return body()
355
+ }
356
+
357
+ internal func _handleParseResult(_ parseResult: Int32) -> Bool {
0 commit comments