4

I came across what appears to be a problem with using boost::format() in multiple threads. The boost format library uses the boost parse library which uses the function std::ctype::narrow() defined in /usr/include/c++/4.8/bits/locale_facets.h (I am using G++ version 4.8).

The narrow() function is not very innocuous. The instance variable _M_narrow is a cache. I found that across threads this cache was being written to and read from at the same time. Having to lock threads to use boost::format seems silly enough to avoid using boost::format which makes me think I must be missing something. Does anyone have any more insight into this problem?

  /**
   *  @brief  Narrow char
   *
   *  This function converts the char to char using the simplest
   *  reasonable transformation.  If the conversion fails, dfault is
   *  returned instead.  For an underived ctype<char> facet, @a c
   *  will be returned unchanged.
   *
   *  This function works as if it returns ctype<char>::do_narrow(c).
   *  do_narrow() must always return the same result for the same input.
   *
   *  Note: this is not what you want for codepage conversions.  See
   *  codecvt for that.
   *
   *  @param __c  The char to convert.
   *  @param __dfault  Char to return if conversion fails.
   *  @return  The converted character.
  */
  char
  narrow(char_type __c, char __dfault) const
  {
    if (_M_narrow[static_cast<unsigned char>(__c)])
      return _M_narrow[static_cast<unsigned char>(__c)];
    const char __t = do_narrow(__c, __dfault);
    if (__t != __dfault)
      _M_narrow[static_cast<unsigned char>(__c)] = __t;
    return __t;
  }
user801824
  • 71
  • 1
  • 4

1 Answers1

3

The definition of _M_narrow is:

mutable char _M_narrow[1 + static_cast<unsigned char>(-1)];

Writing/Reading from this in parallel would in fact be a hazard in general, but in this special case it is fine, as far as I can see. (Please someone correct me if I'm wrong)

  • the data structure is an array, which doesn't get structurally invalidated if written to (opposed to e.g. std::map)
  • the element itself is a char. Writing a char is an atomic operation on all major processors (opposed to int, long or double)
  • the only race condition I see is between the if that checks if the cache is empty and the write operation to the cache. But if the race happens, both threads evaluate do_narrow in parallel and then both write the same result to the cache.

I think this covers all hazardous cases, meaning that this operation is actually thread safe and does not need to get locked.

The only problem with this type of implementation is that clang's thread sanitizer hates it and will always report it as a potential hazard, even though it's safe.

Finomnis
  • 18,094
  • 1
  • 20
  • 27
  • Assuming you have no architecture guarantees about the atomicity of assignment/read on a `char`, two threads writing to the same char is unsafe – Karthik Nishanth Sep 01 '20 at 14:07
  • I'm uncertain what you are talking about ... reading/writing a char is always guaranteed to be atomic on all platforms I know, x86, x86_64, arm. Of course a read-compute-write operation is not atomic, like "i++", but single reads or writes are atomic. Keeping memory coherent between processor cores and caches is the job of the x86 processor, nothing to worry about for the programmer. If you disagree, could you link me a source for this information? – Finomnis Sep 04 '20 at 11:43
  • 1
    For a more detailed discussion about cache coherency, read this thread: https://stackoverflow.com/questions/558848/can-i-force-cache-coherency-on-a-multicore-x86-cpu – Finomnis Sep 04 '20 at 12:00
  • 1
    Practically you're right. `char` is atomic. I don't know if the C++ standard says anything about atomicity of char. That's perhaps why there is a std::atomic_char and a char. The fundamental types in C++ are not guaranteed atomic https://stackoverflow.com/a/35226186/3951920 – Karthik Nishanth Sep 04 '20 at 14:43
  • 1
    You are of course correct, my bad. It would be wise to replace the array of chars with an array of `std::atomic`. On all platforms where chars are already atomic, this will be zero-overhead, most likely only a mem_fence will be added internally. – Finomnis Sep 05 '20 at 15:30