16
#include <iostream>
#include <sstream>
#include <thread>

using namespace std;

int main()
{
    auto runner = []() {
        ostringstream oss;
        for (int i=0; i<100000; ++i)
            oss << i;
    };

    thread t1(runner), t2(runner);
    t1.join(); t2.join();
}

Compile the above code in g++6.2.1, then run it with valgrind --tool=helgrind ./a.out. Helgrind would complain:

==5541== ----------------------------------------------------------------
==5541== 
==5541== Possible data race during read of size 1 at 0x51C30B9 by thread #3
==5541== Locks held: none
==5541==    at 0x4F500CB: widen (locale_facets.h:875)
==5541==    by 0x4F500CB: widen (basic_ios.h:450)
==5541==    by 0x4F500CB: fill (basic_ios.h:374)
==5541==    by 0x4F500CB: std::ostream& std::ostream::_M_insert<long>(long) (ostream.tcc:73)
==5541==    by 0x400CD0: main::{lambda()#1}::operator()() const (43.cpp:12)
==5541==    by 0x4011F7: void std::_Bind_simple<main::{lambda()#1} ()>::_M_invoke<>(std::_Index_tuple<>) (functional:1391)
==5541==    by 0x401194: std::_Bind_simple<main::{lambda()#1} ()>::operator()() (functional:1380)
==5541==    by 0x401173: std::thread::_State_impl<std::_Bind_simple<main::{lambda()#1} ()> >::_M_run() (thread:197)
==5541==    by 0x4EF858E: execute_native_thread_routine (thread.cc:83)
==5541==    by 0x4C31A04: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==5541==    by 0x56E7453: start_thread (in /usr/lib/libpthread-2.24.so)
==5541==    by 0x59E57DE: clone (in /usr/lib/libc-2.24.so)
==5541== 
==5541== This conflicts with a previous write of size 8 by thread #2
==5541== Locks held: none
==5541==    at 0x4EF3B1F: do_widen (locale_facets.h:1107)
==5541==    by 0x4EF3B1F: std::ctype<char>::_M_widen_init() const (ctype.cc:94)
==5541==    by 0x4F501B7: widen (locale_facets.h:876)
==5541==    by 0x4F501B7: widen (basic_ios.h:450)
==5541==    by 0x4F501B7: fill (basic_ios.h:374)
==5541==    by 0x4F501B7: std::ostream& std::ostream::_M_insert<long>(long) (ostream.tcc:73)
==5541==    by 0x400CD0: main::{lambda()#1}::operator()() const (43.cpp:12)
==5541==    by 0x4011F7: void std::_Bind_simple<main::{lambda()#1} ()>::_M_invoke<>(std::_Index_tuple<>) (functional:1391)
==5541==    by 0x401194: std::_Bind_simple<main::{lambda()#1} ()>::operator()() (functional:1380)
==5541==    by 0x401173: std::thread::_State_impl<std::_Bind_simple<main::{lambda()#1} ()> >::_M_run() (thread:197)
==5541==    by 0x4EF858E: execute_native_thread_routine (thread.cc:83)
==5541==    by 0x4C31A04: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==5541==  Address 0x51c30b9 is 89 bytes inside data symbol "_ZN12_GLOBAL__N_17ctype_cE"

It seems that both threads called locale_facet.h:widen which caused data race since there appears no synchronization in this function, even though operator << is called on two different ostringstream object. So I was wondering whether this is really a data race or just a false positive of helgrind.

lz96
  • 2,816
  • 2
  • 28
  • 46
  • 4
    Whatever the standard says, this *should* be threadsafe. – Baum mit Augen Jan 31 '17 at 17:49
  • 3
    The standard states *Concurrent access to a stream object (27.8, 27.9), stream buffer object (27.6), or C Library stream (27.9.2) by multiple threads may result in a data race (1.10) unless otherwise specified (27.4). [ Note: Data races result in undefined behavior (1.10). —end note ]* So my guess is the streams are using some global state in the back end that is not synchronized. Or it is a false positive. My gut says this should be safe. – NathanOliver Jan 31 '17 at 17:53
  • 2
    I think this should be safe according to §17.6.5.9/2: *"A C++ standard library function shall not directly or indirectly access objects (1.10) accessible by threads other than the current thread unless the objects are accessed directly or indirectly via the function’s arguments, including `this`."* So I'd say this is either a non-conforming implementation or a false positive. – Christian Hackl Jan 31 '17 at 18:13
  • 1
    @NathanOliver That's for concurrent accesses to the same object, not for distinct stream objects. – T.C. Jan 31 '17 at 22:27
  • @T.C. I get that. I was just thinking that maybe the streams share something on the back end even though they are separate instances. – NathanOliver Jan 31 '17 at 22:36
  • 5
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77704 – T.C. Jan 31 '17 at 22:38

2 Answers2

1

Update: I admit I didn't fully read the question before answering, so I took it upon myself to research this.

The TL;DR

There are mutable member variables here that could cause race condition. ios has static variables per locale, and these static variables lazy load (initialized when first needed) lookup tables. If you want to avoid concurrency problems, just be sure to initialize the locale prior to joining the threads so that any thread operation is read only to these lookup tables.

The Details

When a stream is initialized it populates a pointer which loads in the correct ctype for the locale (see _M_ctype): https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/basic_ios.h#L273

The error is referring to: https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/locale_facets.h#L875

This could be a race condition if two thread are simultaneously initializing the same locale.

This should be thread-safe (though it may still give error):

// Force ctype to be initialized in the base thread before forking
ostringstream dump;
dump << 1;

auto runner = []() {
    ostringstream oss;
    for (int i=0; i<100000; ++i)
        oss << i;
};

thread t1(runner), t2(runner);
t1.join(); t2.join();
ymmyk
  • 101
  • 9
0

For 2 different streams it's thread safe :)

Amit
  • 229
  • 5
  • 20