4.7.1 C++ target has new Multithread issue

[alainmarcel]

Hi Mike,
I'm doing my developement on 4.7, but I gave a try to the 4.7.1 C++ target:

• Note, there are new gcc compiler warnings, 4.7 was warning free,
• There is a new multithread issue around:

dfa::DFAState *ParserATNSimulator::getExistingTargetState(dfa::DFAState previousD, size_t t) {
dfa::DFAState retval;
auto iterator = previousD->edges.find(t);
retval = (iterator == previousD->edges.end()) ? nullptr : iterator->second;
return retval;
}

I have been trying to play with the read and write locks in the methods of that class, but it seems the problem is pervasive, if I add locks, I can go a bit further but the overall locking mechanism probably lacks complete testing, hence this is not yet multi-thread safe.

Can you address this? My setup is really complex, would be hard to give you a testcase but we could work on that.

Thanks
Alain

[alainmarcel]

An example stack trace:

#0 0x0000000000fd2ed0 in __gnu_cxx::__normal_iterator<std::shared_ptrantlr4::atn::ATNConfig*, std::vector<std::shared_ptrantlr4::atn::ATNConfig, std::allocator<std::shared_ptrantlr4::atn::ATNConfig > > >::__normal_iterator (this=0x7ffff5164300, _i=@0x8: ) at /usr/include/c++/5/bits/stl_iterator.h:741
#1 0x0000000000fd209c in std::vector<std::shared_ptrantlr4::atn::ATNConfig, std::allocator<std::shared_ptrantlr4::atn::ATNConfig > >::begin (this=0x8) at /usr/include/c++/5/bits/stl_vector.h:548
#2 0x0000000000fefb85 in antlr4::atn::ParserATNSimulator::computeReachSet (this=0x7ffff00360f0, closure=0x0, t=36, fullCtx=false) at /home/alain/antlr4/antlr4-4.7.1/runtime/Cpp-Debug/runtime/src/atn/ParserATNSimulator.cpp:453
#3 0x0000000000feef1b in antlr4::atn::ParserATNSimulator::computeTargetState (this=0x7ffff00360f0, dfa=..., previousD=0x7fffe8032f90, t=36) at /home/alain/antlr4/antlr4-4.7.1/runtime/Cpp-Debug/runtime/src/atn/ParserATNSimulator.cpp:269
#4 0x0000000000fee58d in antlr4::atn::ParserATNSimulator::execATN (this=0x7ffff00360f0, dfa=..., s0=0x7fffe8032f90, input=0x7ffff0000bd0, startIndex=0, outerContext=0x7ffff0036230) at /home/alain/antlr4/antlr4-4.7.1/runtime/Cpp-Debug/runtime/src/atn/ParserATNSimulator.cpp:164
#5 0x0000000000fee3d1 in antlr4::atn::ParserATNSimulator::adaptivePredict (this=0x7ffff00360f0, input=0x7ffff0000bd0, decision=23, outerContext=0x7ffff0036230) at /home/alain/antlr4/antlr4-4.7.1/runtime/Cpp-Debug/runtime/src/atn/ParserATNSimulator.cpp:140

[jrobsonchase]

Can confirm - I'm seeing this issue as well after upgrading to 4.7.1 to fix a memory leak issue.

My stack trace is pretty much identical.

Edit: also, my old demo from the first time this issue came up shows the issue after being updated to 4.7.1: https://github.com/jechase/antlr_cpp_threads

[jrobsonchase]

@mike-lischke any updates on this?

[mike-lischke]

Can you try my pending patch #2177? That solved all my multi threading issues so far.

[jrobsonchase]

@mike-lischke Looks like that worked! No threading or memory leak issues :)

[alainmarcel]

Confirmed, works great! Huge memory savings too.
Thanks
Alain

[mike-lischke]

I wonder why you see mem leak fixes, however. There isn't anything in the code that would take care for memory beyond what was there before.

@parrt I think this patch can be merged, now that we have 2 other confirmations it does what it is supposed to do. That way interested developers get a thread safe runtime right with the first download of the branch. And it's a pretty important fix (#2177).

[jrobsonchase]

I saw the memory leaks in 4.7.0 and the threading issue in 4.7.1, where the leaks had (presumably) been fixed. So with this to address the threading issues, all problems are solved! At least as far as I'm aware.

[Krzmbrzl]

@parrt I assume this can be closed now