[
https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13461490#comment-13461490
]
Stefan Teleman commented on STDCXX-1056:
----------------------------------------
The thing I *don't* like about my patch set at all is that it doesn't really
and completely eliminate the cache resize problem, which I strongly suspect is
the root cause of the race conditions, although I do not have a bull's eye
proof for it yet. I have plenty of circumstantial evidence. ;-)
Regardless of the initial cache size we start with, it is still possible that
the cache will have to be resized. Right now, the fix relies on the fact that
it is *unlikely* that a program will ever need more than 32 locales, and the
cache never needs to be resized.
The reason the perfect forwarding patch appears to solve the problem is: the
instantiation of the std::string returned-by-value by the facet member
functions is now different: the do_*() functions return a (cast-to) _CharT*
pointer. Because of this, the std::string returned by value is now constructed
as a deep copy (see the string::string(const _CharT*) 21.3.1/P9 constructor),
which effectively gives ownership of this string to the caller - this
constructor calls traits_type::copy (_C_data, __s, __n);.
For one, the timing difference caused by having to copy the bits (in the
constructor) is enough to alter the run-time behavior. Second, by owning a deep
copy of the returned string, the caller will still have the bits, although the
facet object itself may have long disappeared.
Latest files, diffs and test results are available here:
http://s247136804.onlinehome.us/stdcxx-1056-20120922/
In this latest version I also fixed a bug I had in my original patchset.
The diffs are next-to-impossible to read. It is much easier to just read the
complete files.
The changes I made are in:
locale_body.cpp:
__rw_locale* __rw_locale::_C_manage ( ... );
facet.cpp:
__rw_facet* __rw_facet::_C_manage ( ... );
punct.cpp:
static const void* __rw_get_numpunct ( ... );
static const void* __rw_get_moneypunct ( ... );
In punct.cpp there are two new static inline functions:
static inline const void* __rw_numpunct_switch ( ... );
static inline const void* __rw_moneypunct_switch ( ... );
These two functions are used in replacing the recursive calls present in the
original implementation of __rw_get_numpunct() and __rw_get_moneypunct(). Their
names are horrible.
Recursion is extremely expensive on SPARC (and I strongly suspect on other RISC
machines as well).
I will provide performance data next week. To produce relevant performance
data, I need to do optimized builds. Instrumentation requires non-optimized
debug builds, and the instrumentation part is what I've been focusing on for
the past week.
> std::moneypunct and std::numpunct implementations are not thread-safe
> ---------------------------------------------------------------------
>
> Key: STDCXX-1056
> URL: https://issues.apache.org/jira/browse/STDCXX-1056
> Project: C++ Standard Library
> Issue Type: Bug
> Components: 22. Localization
> Affects Versions: 4.2.1, 4.2.x, 4.3.x, 5.0.0
> Environment: Solaris 10 and 11, RedHat and OpenSuSE Linux, Sun C++
> Compilers 12.1, 12.2, 12.3
> Issue is independent of platform and/or compiler.
> Reporter: Stefan Teleman
> Labels: thread-safety
> Fix For: 4.2.x, 4.3.x, 5.0.0
>
> Attachments: 22.locale.numpunct.mt.out, facet.cpp.diff,
> locale_body.cpp.diff, punct.cpp.diff, runtests-linux32-all.out,
> runtests-linux64-all.out, runtests.out, STDCXX-1056-additional-timings.tgz,
> stdcxx-1056.patch, stdcxx-1056-timings.tgz,
> stdcxx-4.2.x-numpunct-perfect-forwarding.patch,
> stdcxx-4.3.x-numpunct-perfect-forwarding.patch
>
>
> several member functions in std::moneypunct<> and std::numpunct<> return
> a std::string by value (as required by the Standard). The implication of
> return-by-value
> being that the caller "owns" the returned object.
> In the stdcxx implementation, the std::basic_string copy constructor uses a
> shared
> underlying buffer implementation. This shared buffer creates the first
> problem for
> these classes: although the std::string object returned by value *appears* to
> be owned
> by the caller, it is, in fact, not.
> In a mult-threaded environment, this underlying shared buffer can be
> subsequently modified by a different thread than the one who made the initial
> call. Furthermore, two or more different threads can access the same shared
> buffer at the same time, and modify it, resulting in undefined run-time
> behavior.
> The cure for this defect has two parts:
> 1. the member functions in question must truly return a copy by avoiding a
> call to the copy constructor, and using a constructor which creates a deep
> copy of the std::string.
> 2. access to these member functions must be serialized, in order to guarantee
> atomicity
> of the creation of the std::string being returned by value.
> Patch for 4.2.1 to follow.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
