[boost] Threads Review

From: David Abrahams (dave_at_[hidden])
Date: 2002-10-16 10:43:27

I realize this is almost a year too late ;-), but I've been reviewing
the Boost.Threads documentation in detail and I have the following
comments. I also have a whole lot of editorial changes to suggest,
which I'll post separately as a patchset.

* "Thread-safe": this is a term which is well-defined for programs. It
  is /not/ well-defined for classes or functions, though the
  documentation uses it that way repeatedly. The definition
  (definitions.html) says:

      "while some thread-safety violations can be determined
      statically at compile time, many thread-safety errors can only
      be detected at runtime"

  Reasoning/justification, please? I refuse to believe that there
  exist thread-safety problems which can't be detected via careful
  static analysis of the system without proof. Maybe you mean to say
  that some kinds of mistakes can be prevented with compile-time
  constructs, but some mistakes are logic errors which compilers have
  no way to detect?

* Thread State - I think the thread state distinction between "ready"
  and "running" is a false one, since the system can move a thread
  from "ready" to "running" at an arbitrary time, without provocation,
  and the formal description of the library would be a lot simpler if
  you just had a "running" state.

  Also, you use the term "processor" which is not defined either in
  Boost.Threads or in the C++ standard. It is also not detectable by a
  user program AFAICT (by time-sharing/context-switching, an OS can
  implement as many "virtual processors" as it wants), so I suggest
  it's meaningless and should be stricken.

* The definition of "Starvation" gives me pause. Is there any way to
  be more-precise about what's meant by "sufficient"?

* The definition of "accessible from multiple threads" is
  unintuitive:

        "An object [1.8, 1.9] is accessible from multiple threads if
     it is of static storage duration (static, extern) [3.7.1], or if
     a pointer or reference to it is explicitly or implicitly
     dereferenced in multiple threads."

  It would be better to change "is explicitly or implicitly" to "can
  be". It would be even better to say something like:

        "An object [1.8, 1.9] is accessible from multiple threads if
     it is stored in the function object passed to a thread's
     constructor, it is of static storage duration (static, extern)
     [3.7.1], or if a pointer or reference to it is accessible from
     multiple threads."

* Very serious problem, IMO: The second and fourth rows of the table
  in definitions.html seem very wrong to me. To review:

    For the same row of the table, the value of an object accessible
    at the indicated sequence point in thread A will be determinate
    and the same if accessed at or after the indicated sequence point
    in thread B, provided the object is not otherwise modified. In the
    table, the "sequence point at a call" is the sequence point after
    the evaluation of all function arguments [1.9/17], while the
    "sequence point after a call" is the sequence point
    after the copying of the returned value...
    
     The sequence point at a call to a 
     library function which locks a 
     mutex, directly or by waiting for a
     condition variable. 
                                         

</td>                                      
    <td>The sequence point after a call to a library function which unlocks the
      same mutex.</td>
  Row2:
  should say

-- 
           David Abrahams * Boost Consulting
dave_at_[hidden] * http://www.boost-consulting.com