Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> > On Jan 9, 2019, at 9:06 AM, Geert Janssens <geert.gnuc...@kobaltwit.be>
> > wrote:>
> > Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> >>> On Jan 9, 2019, at 6:17 AM, Derek Atkins <de...@ihtfp.com> wrote:
> >>>
> >>> HI,
> >>>
> >>> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
> >>>> I like the idea of caching the system locale for future use. Too bad
> >>>> the
> >>>> std::locale is working so poorly on Windows :(
> >>>>
> >>>> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> >>>>> +const std::locale&
> >>>>> +gnc_get_locale()
> >>>>> +{
> >>>>> + static std::locale cached;
> >>>>> + static bool tried_already = false;
> >>>>
> >>>> If I understand it correctly using static variables makes the function
> >>>> unsuitable for multi-threading, right ?
> >>>
> >>> Not necessarily. There is a race condition on first-use, but beyond
> >>> that
> >>> I don't see a MT issue here. The data is read-only, right? Multiple
> >>> threads could read from the same read-only data simultaneously, so that
> >>> should be fine.
> >
> > It was this first-use race condition I was referring to.
> >
> > Particularly, for thread safety I think setting tried_already = true
> > should
> > happen at the very end of the function, after we're sure cached has a
> > proper value. Otherwise a competing thread could try to get the
> > uninitialized cached value if thread execution of the first thread is
> > interrupted right after tried_already is set true.
> >
> >>> Static data is ont MT-unsafe if it's being changed on a per-call basis
> >>> (e.g. a time_t -> string API returning a static string buffer).
> >>>
> >>>> Any idea how difficult would it be to fix that ?
> >>>
> >>> You could add a mutex around the initialization. That's all I think you
> >>> would need here.
> >>>
> >>>> I know GnuCash is not thread-safe by a long shot and gtk itself is
> >>>> single
> >>>> threaded so it doesn't matter that much.
> >>>>
> >>>> However I silently set a personal goal of changing that sometime. The C
> >>>> code
> >>>> is a lost cause IMO, but it might be worth to keep multi-threading in
> >>>> mind
> >>>> as
> >>>> we gradually convert to c++. In my basic understanding of threading
> >>>> this
> >>>> particular function should not be too hard to make tread safe.
> >>
> >> Right, and I decided against making this the first introduction of mutex
> >> into GnuCash. I think std::atomic
> >> (https://en.cppreference.com/w/cpp/atomic/atomic
> >> <https://en.cppreference.com/w/cpp/atomic/atomic>) is the correct modern
> >> C++ way for a simple case like this.
> >
> > I was hoping indeed modern C++ has some answer to this. This is my first
> > foray into multi-threading by the way so I'm pretty green in this area
> > and wishing to learn more about it.
> >
> > In this particular case would declaring the cached and tried_already as
> > atomic be sufficient to make this thread safe ?
> >
> > It seems to me this would still allow multiple threads to go in and run
> > the
> > initialization code for setting cached. Given they all should end up
> > setting cached to the same locale it's probably not dangerous to the
> > application, only potentially running the same code multiple times where
> > only once would be sufficient.
>
> My knowledge of concurrency is pretty dated. I had an operating system
> course 30 years ago that included a discussion of concurrency, and some
> parts of Glib are thread-safe, so I’ve bounced up against it doing
> maintenance a few times. Just mutex and semaphore stuff.
>
> I haven’t even yet read the concurrency chapter in Josuttis, but my
> understanding of std::atomic is that a std::atomic variable magically
> eliminates races and has certain operations (construction and operator=
> among them) that are guaranteed to be, well, atomic: The compiler will
> always create a contiguous uninterruptible sequence of machine instructions
> for atomic operations. There are also ways to order execution of some
> functions with std::memory_model that can make locks (i.e. mutexes and
> semaphores) unnecessary, and the compiler is able to figure out when that’s
> true and when it isn’t and to take care of the locking without the
> programmer needing to explicitly code it.
>
> It’s not something I think worth worrying about now. GnuCash is very far
> away from being multi-threaded. Besides, the natural home for the
> instantiation of the cached C++ local is in main(), after all of the
> environment options are parsed but before the GUI is loaded or the first
> session started. This isn’t the function to worry about concurrent access.
>
Fair enough. Though I'd think it should happen in libgnucash initialization at
some point rather than in the gui code. But that's just a matter of sorting
out all startup configuration we do at some point and estimate which part is
needed for libgnucash and which part is only for the gui code.
Geert
_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
