Re: [jira] [Closed] (STDCXX-1056) std::moneypunct and std::numpunct implementations are not thread-safe
On Tue, Sep 25, 2012 at 10:58 PM, Martin Sebor wrote: > As for Stefan's patches, the biggest problem I see there > (besides the above) is that they introduce changes that > appear to be unrelated to the problem without providing > a satisfactory rationale. The established stdcxx bug > fixing process is to create as small a patch as possible > that does nothing but address the problem. This is > particularly important when we don't have ready access > to test the patch with all the targeted compilers and > on all the targeted operating systems (to minimize the > risk of incidental breakage and to make it as easy as > possible to root cause possible regressions. There is a very simple rationale for the complexity of the second version of the thread safety patch: you objected to the first version of the patch, the one I had originally submitted for 1056, because it was too coarse. I am talking about the patch which applied the mutex lock to every public interface in std::numpunct<> and std::moneypunct(). This patch is also the absolute simplest one in terms of code changes, and it does not affect binary compatibility at all. The complexity of the second version of the patch is a consequence of your request to try finding a version which is not as coarse, which performs a more granular and shorter-lasting mutex locking, and which preserves binary compatibility. That's what the second patch version does. However, it comes with code complexity side-effects. It is also slightly better than the first one in terms of performance. One of the ways by which this somewhat better performance with more granular locking was achieved was by increasing the size of the locale cache, and by eliminating automatic and unnecessary cache resizing operations, which involve copying and deletions. This change simply eliminates code. A second way by which this was achieved was by unwinding recursion. The usefulness and/or necessity of using recursion in __rw_get_numpunct() and __rw_get_moneypunct() was dubious anyway. Because of the existing design and implementation of the locale caching, there isn't much wiggle room, if preserving binary compatibility is a hard requirement. -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: [jira] [Closed] (STDCXX-1056) std::moneypunct and std::numpunct implementations are not thread-safe
On 09/25/2012 06:08 PM, Wojciech Meyer wrote: Hi guys, I think the consensus about this bug is that everybody wants to fix it. Stefan has a patch but I got completely lost what is the exact reason of not applying it. It would be a shame to stop at this point. From my POV, there's no disagreement that there is a bug in the thread safety of the facets or its cause, or a lack of desire to fix it. I think the main reason for the lack of consensus on a patch is that we don't yet have a good sense of the performance impact of the proposed approaches or an understanding of the differences in the timings. As for Stefan's patches, the biggest problem I see there (besides the above) is that they introduce changes that appear to be unrelated to the problem without providing a satisfactory rationale. The established stdcxx bug fixing process is to create as small a patch as possible that does nothing but address the problem. This is particularly important when we don't have ready access to test the patch with all the targeted compilers and on all the targeted operating systems (to minimize the risk of incidental breakage and to make it as easy as possible to root cause possible regressions. Martin Liviu Nicoara writes: On 9/25/12 7:56 PM, Stefan Teleman (JIRA) wrote: [ https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Stefan, I don't think it's ok to close this bug. The race conditions are there and we have not come to a completely satisfactory conclusion on how to deal with them, or even if we should deal with them. Whichever it is we gotta keep this discussion open. I sure hope you want to be a part of it. FWIW, I have spent quite some time looking at your proposed patch and I am going to reopen the incident. If I can. Liviu Stefan Teleman closed STDCXX-1056. -- Resolution: Won't Fix Bug will not be fixed. Upstream refuses to acknowledge the existence of the bug in spite of objective evidence to the contrary. 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 -- Wojciech Meyer http://danmey.org
Re: [jira] [Closed] (STDCXX-1056) std::moneypunct and std::numpunct implementations are not thread-safe
On Tue, Sep 25, 2012 at 8:05 PM, Liviu Nicoara wrote: > On 9/25/12 7:56 PM, Stefan Teleman (JIRA) wrote: >> >> >> [ >> https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >> ] > > > Stefan, > > I don't think it's ok to close this bug. The race conditions are there and > we have not come to a completely satisfactory conclusion on how to deal with > them, or even if we should deal with them. Whichever it is we gotta keep > this discussion open. I sure hope you want to be a part of it. > > FWIW, I have spent quite some time looking at your proposed patch and I am > going to reopen the incident. If I can. I am done wasting my time on trying to convince this project that it should fix its severe bugs. If and when this project decides to abandon the adolescent attitude which appears to be one of its primary characteristics, let me know. -- Stefan Teleman KDE e.V. stefan.tele...@gmail.com
Re: [jira] [Closed] (STDCXX-1056) std::moneypunct and std::numpunct implementations are not thread-safe
Hi guys, I think the consensus about this bug is that everybody wants to fix it. Stefan has a patch but I got completely lost what is the exact reason of not applying it. It would be a shame to stop at this point. Liviu Nicoara writes: > On 9/25/12 7:56 PM, Stefan Teleman (JIRA) wrote: >> >> [ >> https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >> ] > > Stefan, > > I don't think it's ok to close this bug. The race conditions are there > and we have not come to a completely satisfactory conclusion on how to > deal with them, or even if we should deal with them. Whichever it is > we gotta keep this discussion open. I sure hope you want to be a part > of it. > > FWIW, I have spent quite some time looking at your proposed patch and > I am going to reopen the incident. If I can. > > Liviu > >> >> Stefan Teleman closed STDCXX-1056. >> -- >> >> Resolution: Won't Fix >> >> Bug will not be fixed. Upstream refuses to acknowledge the existence of the >> bug in spite of objective evidence to the contrary. >> >>> 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 >> > -- Wojciech Meyer http://danmey.org
Re: [jira] [Closed] (STDCXX-1056) std::moneypunct and std::numpunct implementations are not thread-safe
On 9/25/12 7:56 PM, Stefan Teleman (JIRA) wrote: [ https://issues.apache.org/jira/browse/STDCXX-1056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Stefan, I don't think it's ok to close this bug. The race conditions are there and we have not come to a completely satisfactory conclusion on how to deal with them, or even if we should deal with them. Whichever it is we gotta keep this discussion open. I sure hope you want to be a part of it. FWIW, I have spent quite some time looking at your proposed patch and I am going to reopen the incident. If I can. Liviu Stefan Teleman closed STDCXX-1056. -- Resolution: Won't Fix Bug will not be fixed. Upstream refuses to acknowledge the existence of the bug in spite of objective evidence to the contrary. 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
