Return-Path: X-Original-To: apmail-stdcxx-dev-archive@www.apache.org Delivered-To: apmail-stdcxx-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id A791AD304 for ; Thu, 6 Sep 2012 23:31:44 +0000 (UTC) Received: (qmail 40179 invoked by uid 500); 6 Sep 2012 23:31:44 -0000 Delivered-To: apmail-stdcxx-dev-archive@stdcxx.apache.org Received: (qmail 40136 invoked by uid 500); 6 Sep 2012 23:31:44 -0000 Mailing-List: contact dev-help@stdcxx.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@stdcxx.apache.org Delivered-To: mailing list dev@stdcxx.apache.org Received: (qmail 40123 invoked by uid 99); 6 Sep 2012 23:31:44 -0000 Received: from minotaur.apache.org (HELO minotaur.apache.org) (140.211.11.9) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 06 Sep 2012 23:31:44 +0000 Received: from localhost (HELO 002128155014.mbb.telenor.dk) (127.0.0.1) (smtp-auth username lnicoara, mechanism plain) by minotaur.apache.org (qpsmtpd/0.29) with ESMTP; Thu, 06 Sep 2012 23:31:44 +0000 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Apple Message framework v1278) Subject: Re: STDCXX-1056 [was: Re: STDCXX forks] From: Liviu Nicoara In-Reply-To: <5047B01E.2060506@gmail.com> Date: Thu, 6 Sep 2012 19:31:42 -0400 Content-Transfer-Encoding: quoted-printable Message-Id: References: <40394653-8FCC-4D04-A108-2C650AF8F95B@hates.ms> <5045E764.9090607@hates.ms> <595887D2-6E42-4BC4-AF69-085AE4BA8A7D@hates.ms> <5046BDC1.3020400@gmail.com> <50476748.9040301@gmail.com> <50479CA6.8010306@hates.ms> <5047A561.5050606@gmail.com> <5047A926.7070308@hates.ms> <5047B01E.2060506@gmail.com> To: dev@stdcxx.apache.org X-Mailer: Apple Mail (2.1278) On Sep 5, 2012, at 4:03 PM, Martin Sebor wrote: > On 09/05/2012 01:33 PM, Liviu Nicoara wrote: >> On 09/05/12 15:17, Martin Sebor wrote: >>> On 09/05/2012 12:40 PM, Liviu Nicoara wrote: >>>> On 09/05/12 14:09, Stefan Teleman wrote: >>>>> On Wed, Sep 5, 2012 at 10:52 AM, Martin Sebor = wrote: >>>>> [...] >>>>> OK so I did a little bit of testing, after looking at the *right* >>>>> __rw_guard class. :-) >>>>>=20 >>>>> I changed the std::numpunct class thusly: >>>>> [...] >>>>=20 >>>> I am afraid this would be unsafe, too (if I said otherwise earlier = I was >>>> wrong). [...] Thoughts? >>>=20 >>> You're right, there's still a problem. We didn't get the double >>> checked locking quite right. >>=20 >> I wish for simplicity: eliminate the lazy initialization, and fully >> initialize the facet in the constructor. Then we'd need no locking on >> copying its data (std::string takes care of its own copying). >=20 > I'm not sure how easily we can do that. Almost all of locale > is initialized lazily. Some of the layers might depend on the > facets being initialized lazily as well. This was a deliberate > design choice. One of the constraints was to avoid dynamic > initialization or allocation at startup. [...] There would be a performance degradation. IMHO, it would be minor and = would simplify the code considerably. After finally being able to reproduce the defect with SunPro 12.3 on = x86_64 I tried to remove the lazy initialization and a (smaller) test = case now passes. I am attaching the program and the numpunct patch.=20 Will continue to look into it. Although STDCXX does not have an = implementation of the atomics library we could probably use the = existing, unexposed, atomics API to implement a more robust lazy = initialization.=20 Liviu $ cat tests/localization/t.cpp; svn diff #include #include #include #include #define MAX_THREADS 16 #define MAX_LOOPS 1000 static bool hold =3D true; extern "C" { static void*=20 f (void* pv) { std::numpunct const& fac =3D=20 *reinterpret_cast< std::numpunct* > (pv); =20 while (hold) ;=20 for (int i =3D 0; i !=3D MAX_LOOPS; ++i) { std::string const grp =3D fac.grouping (); } return 0; } } int main (int argc, char** argv) { std::locale const loc =3D std::locale (argv [1]); std::numpunct const& fac =3D std::use_facet > (loc); pthread_t tid [MAX_THREADS] =3D { 0 }; for (int i =3D 0; i < MAX_THREADS; ++i) { if (pthread_create (tid + i, 0, f, (void*)&fac))=20 exit (-1); } sleep (1); hold =3D false; for (int i =3D 0; i < MAX_THREADS; ++i) { if (tid [i]) pthread_join (tid [i], 0); } return 0; } Index: include/loc/_numpunct.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- include/loc/_numpunct.h (revision 1381575) +++ include/loc/_numpunct.h (working copy) @@ -61,24 +61,40 @@ struct numpunct: _RW::__rw_facet string_type; =20 _EXPLICIT numpunct (_RWSTD_SIZE_T __ref =3D 0) - : _RW::__rw_facet (__ref), _C_flags (0) { } + : _RW::__rw_facet (__ref) { + _C_grouping =3D do_grouping (); + _C_truename =3D do_truename (); + _C_falsename =3D do_falsename (); + _C_decimal_point =3D do_decimal_point (); + _C_thousands_sep =3D do_thousands_sep (); + } =20 virtual ~numpunct () _RWSTD_ATTRIBUTE_NOTHROW; =20 // 22.2.3.1.1, p1 - char_type decimal_point () const; + char_type decimal_point () const { + return _C_decimal_point; + } =20 // 22.2.3.1.1, p2 - char_type thousands_sep () const; + char_type thousands_sep () const { + return _C_thousands_sep; + } =20 // 22.2.3.1.1, p3 - string grouping () const; + string grouping () const { + return _C_grouping; + } =20 // 22.2.3.1.1, p4 - string_type truename () const; + string_type truename () const { + return _C_truename; + } =20 // 22.2.3.1.1, p4 - string_type falsename () const; + string_type falsename () const { + return _C_falsename; + } =20 static _RW::__rw_facet_id id; =20 @@ -112,15 +128,13 @@ protected: =20 private: =20 - int _C_flags; // bitmap of "cached data valid" = flags - string _C_grouping; // cached results of virtual = members + string _C_grouping; string_type _C_truename; string_type _C_falsename; char_type _C_decimal_point; char_type _C_thousands_sep; }; =20 - #ifndef _RWSTD_NO_SPECIALIZED_FACET_ID =20 _RWSTD_SPECIALIZED_CLASS @@ -134,95 +148,6 @@ _RW::__rw_facet_id numpunct::id # endif // _RWSTD_NO_WCHAR_T #endif // _RWSTD_NO_SPECIALIZED_FACET_ID =20 - -template -inline _TYPENAME numpunct<_CharT>::char_type -numpunct<_CharT>::decimal_point () const -{ - if (!(_C_flags & _RW::__rw_dp)) { - - numpunct* const __self =3D _RWSTD_CONST_CAST (numpunct*, this); - - // [try to] get the decimal point first (may throw) - // then set a flag to avoid future initializations - __self->_C_decimal_point =3D do_decimal_point (); - __self->_C_flags |=3D _RW::__rw_dp; - } - - return _C_decimal_point; -} - - -template -inline _TYPENAME numpunct<_CharT>::char_type -numpunct<_CharT>::thousands_sep () const -{ - if (!(_C_flags & _RW::__rw_ts)) { - - numpunct* const __self =3D _RWSTD_CONST_CAST (numpunct*, this); - - // [try to] get the thousands_sep first (may throw) - // then set a flag to avoid future initializations - __self->_C_thousands_sep =3D do_thousands_sep (); - __self->_C_flags |=3D _RW::__rw_ts; - } - - return _C_thousands_sep; -} - - -template -inline string numpunct<_CharT>::grouping () const -{ - if (!(_C_flags & _RW::__rw_gr)) { - - numpunct* const __self =3D _RWSTD_CONST_CAST (numpunct*, this); - - // [try to] get the grouping first (may throw) - // then set a flag to avoid future initializations - __self->_C_grouping =3D do_grouping (); - __self->_C_flags |=3D _RW::__rw_gr; - } - - return _C_grouping; -} - - -template -inline _TYPENAME numpunct<_CharT>::string_type -numpunct<_CharT>::truename () const -{ - if (!(_C_flags & _RW::__rw_tn)) { - - numpunct* const __self =3D _RWSTD_CONST_CAST (numpunct*, this); - - // [try to] get the true name first (may throw) - // then set a flag to avoid future initializations - __self->_C_truename =3D do_truename (); - __self->_C_flags |=3D _RW::__rw_tn; - } - - return _C_truename; -} - - -template -inline _TYPENAME numpunct<_CharT>::string_type -numpunct<_CharT>::falsename () const -{ - if (!(_C_flags & _RW::__rw_fn)) { - - numpunct* const __self =3D _RWSTD_CONST_CAST (numpunct*, this); - - // [try to] get the false name first (may throw) - // then set a flag to avoid future initializations - __self->_C_falsename =3D do_falsename (); - __self->_C_flags |=3D _RW::__rw_fn; - } - - return _C_falsename; -} - // #endif _RWSTD_NO_EXT_NUMPUNCT_PRIMARY =20