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

Swift Target Crashes with Multi-Threading #3271

Closed
2 tasks done
rmehta33 opened this issue Sep 8, 2021 · 4 comments · Fixed by #3276
Closed
2 tasks done

Swift Target Crashes with Multi-Threading #3271

rmehta33 opened this issue Sep 8, 2021 · 4 comments · Fixed by #3276
Milestone

Comments

@rmehta33
Copy link

rmehta33 commented Sep 8, 2021

Reproducing Error

Running via Xcode for iOS 13
Project Swift Version: 4.2
Compiled swift target from 4.9.2 release

Grammar

grammar math;

NUMBER : [0-9] ;
WS : [ \r\n\t] + -> skip ;

operation : l=NUMBER op='+' r=NUMBER  ;

Swift Code

for _ in 0...10 {
    DispatchQueue.global().async {
        let chars = ANTLRInputStream("1+1")
        let lexer = mathLexer(chars)
        
        let tokenStream = CommonTokenStream(lexer)
        let parser = try! mathParser(tokenStream)
        
        let node = try? parser.operation()
    }
}

What Happened

The program crashes in LexerATNSimulator.swift on line 721 with EXC_BAD_ACCESS. There was a double free at some point in the stack.

Any help is greatly appreciated! Hopefully this is just user error.

@martinvw
Copy link
Contributor

Hello @rmehta33, I fear that I'm bumping in the same issue, but I did not yet have a reproduction path, does you Backtrace look a bit like this:

0  CoreFoundation               	0x18a642708 __exceptionPreprocess + 220
1   libobjc.A.dylib               	0x19f14c7a8 objc_exception_throw + 59
2   CoreFoundation                	0x18a545bfc -[NSObject+ 183292 (NSObject) doesNotRecognizeSelector:] + 143
3   CoreFoundation                	0x18a645260 ___forwarding___ + 1443
4   CoreFoundation                	0x18a647560 _CF_forwarding_prep_0 + 95
5   libswiftCore.dylib            	0x18e26aa8c __CocoaDictionary.lookup+ 789132 (_:) + 35
6   *redacted*                  	0x10e204dd0 specialized Dictionary.subscript.getter + 79
7   *redacted*                  	0x10e1aecac closure #1 in LexerATNSimulator.addDFAState+ 437420 (_:) + 91
8   *redacted*                  	0x10e1ac26c LexerATNSimulator.addDFAState+ 426604 (_:) + 767
9   *redacted*                  	0x10e1abb70 LexerATNSimulator.matchATN+ 424816 (_:) + 187
10  *redacted*                  	0x10e1ab754 LexerATNSimulator.match+ 423764 (_:_:) + 359
11  *redacted*                  	0x10e2007fc Lexer.nextToken+ 772092 () + 795

If so that would be awesome I'm already looking at this for days without a reproduction path, I did not think I was running multiple threads but there might also be a small issue there. Thanks for your input.

@martinvw
Copy link
Contributor

martinvw commented Sep 10, 2021

Thinking out loud I think I see (part of) the problem.

  1. the decisionToDFA field inside LexerATNSimulator is private

public private(set) final var decisionToDFA: [DFA]

  1. But not constructed internally but received

public init(_ recog: Lexer?, _ atn: ATN,
_ decisionToDFA: [DFA],
_ sharedContextCache: PredictionContextCache) {
self.decisionToDFA = decisionToDFA
self.recog = recog
super.init(atn, sharedContextCache)
}

  1. from the generated parser

override <accessLevelNotOpen(parser)>
init(_ input:TokenStream) throws {
RuntimeMetaData.checkVersion("4.9.2", RuntimeMetaData.VERSION)
try super.init(input)
_interp = ParserATNSimulator(self,<p.name>._ATN,<p.name>._decisionToDFA, <parser.name>._sharedContextCache)
}

  1. In that class, it is a static field

internal static var _decisionToDFA: [DFA] = {
var decisionToDFA = [DFA]()
let length = <parser.name>._ATN.getNumberOfDecisions()
for i in 0..\<length {
<!// decisionToDFA[i] = DFA(<parser.name>._ATN.getDecisionState(i)!, i);!>
decisionToDFA.append(DFA(<parser.name>._ATN.getDecisionState(i)!, i))
}
return decisionToDFA
}()

  1. However the Mutex guarding access

return dfaStatesMutex.synchronized {
if let existing = dfa.states[proposed] {
return existing
}
let newState = proposed
newState.stateNumber = dfa.states.count
configs.setReadonly(true)
newState.configs = configs
dfa.states[newState] = newState
return newState
}

  1. Is defined on class level:

///
/// mutex for DFAState change
///
private let dfaStateMutex = Mutex()
///
/// mutex for changes to all DFAStates map
///
private let dfaStatesMutex = Mutex()

I do believe that this is likely to be the cause of the issue, but please correct me if I'm wrong!

@ewanmellor, @ericvergnaud, and/or other (Swift) maintainers your opinion is greatly appreciated

Edit ----

I did a quick comparison with Java which has comparable code, however, the synchronized works there on top of the object reference so similar problem do not occur there, I fear we have to pass in the mutex-es externally as well.

@rmehta33
Copy link
Author

Hello @rmehta33, I fear that I'm bumping in the same issue, but I did not yet have a reproduction path, does you Backtrace look a bit like this:

0  CoreFoundation               	0x18a642708 __exceptionPreprocess + 220
1   libobjc.A.dylib               	0x19f14c7a8 objc_exception_throw + 59
2   CoreFoundation                	0x18a545bfc -[NSObject+ 183292 (NSObject) doesNotRecognizeSelector:] + 143
3   CoreFoundation                	0x18a645260 ___forwarding___ + 1443
4   CoreFoundation                	0x18a647560 _CF_forwarding_prep_0 + 95
5   libswiftCore.dylib            	0x18e26aa8c __CocoaDictionary.lookup+ 789132 (_:) + 35
6   *redacted*                  	0x10e204dd0 specialized Dictionary.subscript.getter + 79
7   *redacted*                  	0x10e1aecac closure #1 in LexerATNSimulator.addDFAState+ 437420 (_:) + 91
8   *redacted*                  	0x10e1ac26c LexerATNSimulator.addDFAState+ 426604 (_:) + 767
9   *redacted*                  	0x10e1abb70 LexerATNSimulator.matchATN+ 424816 (_:) + 187
10  *redacted*                  	0x10e1ab754 LexerATNSimulator.match+ 423764 (_:_:) + 359
11  *redacted*                  	0x10e2007fc Lexer.nextToken+ 772092 () + 795

If so that would be awesome I'm already looking at this for days without a reproduction path, I did not think I was running multiple threads but there might also be a small issue there. Thanks for your input.

Yes!

@martinvw
Copy link
Contributor

@rmehta33 I created a PR: #3276

@parrt parrt added this to the 4.9.3 milestone Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants