Hi, > The setlocale() thread-safeness problems are well known, but there is also a > rather significant lack of a decent widely adopted workaround.
I know, but what I want to say is that according to this page [1] the rrd_update_r function is said to be thread-save but it isn't. Hence, if I had not stumbled across the first point (aka the memory bug) I would not have noticed this one. I would never have assumed that rrd_update_r uses setlocale. (Perhaps sometimes later in the future after I would have observed strange behaviour.) That's why I asked for a big warning at [1]. I do not know yet, if setlocale is the only problem with rrd_update_r, but at least I see a bug fix for this one. There is no need to use setlocale at all, if there was an interface like rrd_update_r( cont char* rrdFile, time_t ts, size_t argc, const char* desc[], const T* val[] ) for T being a numeric type. This way one could avoid the conversion to and from a string at all. The calling application is responsible of providing the correct datatype. That should even simplify rrd_update_r because there is no need to parse an argument of the pattern "<ts>:<val 1>:...:<val n>". Regards, Matthias [1] http://oss.oetiker.ch/rrdtool/prog/rrdthreads.en.html 2014-06-16 1:47 GMT+02:00 Donovan Baarda <[email protected]>: > The setlocale() thread-safeness problems are well known, but there is also a > rather significant lack of a decent widely adopted workaround. This is the > problem with many of the standard C libraries... they pre-date threading, > and there was never consensus/standardizing on any thread safe alternative. > > For any thread-safe alternative to work, everything needs to use it, but > without a standard, you can pretty much guarantee that something you link > will not use it, and some platforms won't even have it. > > So instead, your best option is often to just declare your code > non-thread-safe and recommend subprocesses instead... but then the > inter-process communication libs are not as well developed as the threading > libs. > > Either that or completely embrace something like glib which replaces nearly > everything in the standard C libs and only use other libs that also use > glib. When you do this you also have to accept that your code doesn't look > like normal C any more, and you might have some platform restrictions. > > > > On 16 June 2014 08:39, Matthias Nagel <[email protected]> wrote: >> >> Hello, >> thanks. In the meantime I patched my Debian packages locally. But I ran >> into another race condition. rrd_update_r() isn't thread-save, because the C >> locale is an application wide variable. Assume one has rrdlib (A) and some >> other library (B) and the execution order is as follows: >> >> (A1) old_locale = setlocale(...) >> (B1) old_locale = setlocale(...) >> (A2) // do some locale-dependent stuff >> (A3) setlocale( old_locale ) >> (B2) // do some locale-dependent stuff >> (B3) setlocale( old_locale ) >> >> Here, library B can be any library that also sets the global C locale >> within a different thread context. In the best case some strings are >> misinterpreted, in the worst case the memory gets corrupted :-( At the >> moment, I wrote a work-around by using an application wide mutex that must >> be locked by any thread that wants to call any library that might change the >> global C locale. But of course this isn't very nice. >> >> Are there any chances that the rrd_update_r function (and relatives) will >> be rewritten? For example, C++ locales are bounded to a specific stream and >> are not global. At least there should be big, BIG warning at >> http://oss.oetiker.ch/rrdtool/prog/rrdthreads.en.html that the C locale is >> subject to a race condition. >> >> Regards, Matthias >> >> Am Sonntag, 15. Juni 2014, 22:37:01 schrieb Tobias Oetiker: >> > Hi Matthias, >> > >> > yes you are right ... we fixed this in master, but not in the 1.4 >> > branch ... it is now ... >> > >> > cheers >> > tobi >> > >> > Today Matthias Nagel wrote: >> > >> > > Hello, >> > > >> > > I am writing a multi-threaded C++ application that uses rrdlib >> > > natively by calling rrd_update_r(). If I compile without optimazations >> > > and >> > > enable -ggdb everything seems to work fine. As soon as I switch to -O2 >> > > and >> > > disable -ggdb my apllication crashes at runtime. >> > > >> > > If it crashes the output is either >> > > >> > > *** glibc detected *** rrdtool: <something> >> > > >> > > or >> > > >> > > expected timestamp not found in data source from <input> >> > > >> > > but <input> is not the string that was given to rrd_update_r but >> > > unreadable garbage. Obviously, it is a memory corruption problem. >> > > Therefore, >> > > I ran the application under valgrind and I noticed that the problems >> > > comes >> > > from inside of the rrdlib. The message is >> > > >> > > >> > > ==11724== Invalid read of size 1 >> > > ==11724== at 0x4C2A051: __GI_strcmp (mc_replace_strmem.c:712) >> > > ==11724== by 0x5A4FF7F: setlocale (setlocale.c:210) >> > > ==11724== by 0x505D06B: _rrd_update (rrd_update.c:982) >> > > ==11724== Address 0x9deb0d0 is 0 bytes inside a block of size 12 >> > > free'd >> > > ==11724== at 0x4C27D4E: free (vg_replace_malloc.c:427) >> > > ==11724== by 0x5A4FCBD: setname (setlocale.c:173) >> > > ==11724== by 0x5A500B0: setlocale (setlocale.c:417) >> > > ==11724== by 0x505D02D: _rrd_update (rrd_update.c:974) >> > > >> > > Let's have a look at it: >> > > >> > > rrd_update.c:973: old_locale = setlocale(LC_NUMERIC, NULL); >> > > rrd_update.c:974: setlocale(LC_NUMERIC, "C"); >> > > rrd_update.c:982: setlocale(LC_NUMERIC, old_locale); >> > > >> > > The problem is obvious. The variable "old_locale" that is used at the >> > > 3rd line was assigned at the 1st line. But the 2nd call to "setlocale" >> > > freed >> > > the return value of the first call. According to the man pages the return >> > > value is a pointer to static memory and freed/allocated on every call. >> > > Actually the 2nd line (974) should be ommited and it should be >> > > >> > > rrd_update.c:973: old_locale = setlocale(LC_NUMERIC, "C" ); >> > > rrd_update.c:974: // deleted >> > > rrd_update.c:982: setlocale(LC_NUMERIC, old_locale); >> > > >> > > Why this double call to "setlocale" anyway? >> > > >> > > Best regards, Matthias >> > > >> > > >> > >> > >> >> -- >> Matthias Nagel >> Parkstraße 27 >> 76131 Karlsruhe >> >> Festnetz: +49-721-96869289 >> Mobil: +49-151-15998774 >> e-Mail: [email protected] >> ICQ: 499797758 >> Skype: nagmat84 >> >> _______________________________________________ >> rrd-users mailing list >> [email protected] >> https://lists.oetiker.ch/cgi-bin/listinfo/rrd-users >> > > > > -- > Donovan Baarda <[email protected]> -- Matthias Nagel Parkstraße 27 76131 Karlsruhe Festnetz: +49-721-96869289 Mobil: +49-151-15998774 e-Mail: [email protected] ICQ: 499797758 Skype: nagmat84 _______________________________________________ rrd-users mailing list [email protected] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-users
