Hi Georg,
Warning: I haven't read LyX source code for a long long time :-)
On 19/11/2014 22:43, Georg Baum wrote:
Hi,
while investigating http://www.lyx.org/trac/ticket/9336 I found out a
fundamental problem with our multithreaded export: The GNU libstdc++ gives
only a relatively weak thread-safety guarantee about std::basic string. It
is not completely thread-safe in the POSIX sense. The reason for this lies
in the employed copy-on-write technique, which is not used in other STL
implementations such as the one in clang or MSVC. It is even explicitly
forbidden in the new C++11 standard, and the gcc folks already acknowledged
that they will get rid of it, but since this is an ABI incompatible change
they want to wait until they have several such changes to do all in one go.
For details please see the excellent gcc bug report by James Kanze (who is a
C++ expert) at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334,
especially https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334#c21, which
describes almost exactly the situation we have with the document language.
This looks all rather theoretical, but in practice it means that it is quite
easy to produce crashes if you use LyX built with #define EXPORT_in_THREAD 1
and gcc, and start an export, depending on the document (master-child setups
seem to be much more vulnerable). The thing which seems to be most affected
is Language::babel_, which is copied a lot, but problems could arise from
all std::string and docstring variables.
I am not questioning your reasoning but I think STL thread safety
limitation has limited impact in our case. AFAIR, the Buffer is cloned
in in the current and main thread so we have no race condition here.
After that the cloned buffer shares nothing with the original Buffer and
can thus be used safely in another thread.
What do we do now? I can think of several alternatives:
1) Do nothing, because my findings are incorrect. I don't believe that of
course, but I would appreciate it if somebody would try to find flaws in my
reasoning.
I believe that there could be many reasons that could cause a crash
before incriminating the stl string, etc:
* Gui elements (message passing?)
* Graphics preview elements
2) Disable EXPORT_in_THREAD if GNU libstdc++ is found. This is an easy and
safe solution, with the disadvantage that it removes a feature (but I think
that this is much better than risking crashes).
3) Derive an own class lyx::basic_string from std::basic_string which
circumvents copy-on-wrtite in the copy-constructor and assignment operator
by creating the copy from c_str(). Then use it
namespace lyx {
typedef basic_string<char_type> docstring;
typedef basic_string<char> string;
}
and replace all occurances of std::string with lyx::string. This would of
course need a lot of testing, but I believe it could work.
4) Do not use std::basic_string at all, but an own implementation which
could be stolen from STLport or clang (if the licenses permit that and are
compatible to the GPL, I did not check that). However, I believe that this
would create lots of interface problems with other libraries.
Does anybody see any other solution? I think for the stable branch we do not
really have any choice and need to implement 2).
Why not compile with c++11 option? I guess basic_string won't do
copy-on-write if compiled this way?
The other question is: What can we learn for the future? AFAIK
EXPORT_in_THREAD was developed on windows, and it was probably well tested
on that platform. However, one basic lesson to learn is that such important
infrastructure changes need thorough testing on all supported platforms, and
I don't think this did happen in this case. Despite qt hides many
differences for us there are still a lot of platform specific issues left.
Well, IIRC I developed buffer cloning and initial threaded export using
Linux :-)
Maybe Peter Kuemmel continued the work under Windows but I don't think so...
Thanks Georg, that was nice to discuss this.
Abdel.
