Re: Review: Boost.Threads

From: Eric Friedman (ebf_at_[hidden])
Date: 2001-09-15 00:14:31

I am admittedly rather new in the field of multithreaded/concurrent
programming.  But for what it's worth, having observed the debate for
the past few days, I must say that I agree with the use of free
functions rather than member functions for a number of operations.

(I apologize in advance for not directly quoting statements made by
others in my remarks below; as I noted above, I have been observing this
conversation for a number of days, and doing so would have required
drawing from a large number of messages by various people.)

My position should become clear through the following statements:

* thread::join() --

From my own experience looking over the Boost.Threads library, I can say
it took me a while to realize that the join operation involved the
current thread at all.  I openly admit that it was due to my own lack of
understanding of the conceptual nature of a join operation, but seeing
the current syntax made wonder: "join the thread to what?" (which I
don't think is the desired effect the library should have on entry-level
users).

If there had been a free function boost::wait(thread) with the semantic
understanding that the function was *wait*ing for the provided thread to
complete and subsequently cleanup before returning (which is all a join
operation does as far as I can tell -- though I am likely to be mistaken
on this one), I feel that I would not have experienced this confusion.

Indeed, even now that I understand (or at least I think I understand)
what a join operation does, I do not see any potential confusion in
boost::wait(thread) meaning "wait until the thread completes and cleans
up."  Please tell me that I am missing something if I am.

(Another possible alternative, which I do not particularly favor but
would have solved the problem of lessening confusion for at least me and
probably others, would be to rename the function to the more verbose
member function thread::join_to_current() or something along those
lines.  And of course the logical abstraction of this particular line of
thinking would be to accept a thread argument that would indicate which
thread to join to, but I don't see the need for such an abstraction
arising frequently, if ever, nor do I think it would be easy, nor
reasonable, to implement.)

* condition::wait(scoped_lock) --

I had less difficulty in understanding what this function was supposed
to do (probably because the action associated with the verb "to wait"
conjures more familiar programming analogies than does "to join").  But
I nevertheless agree with the other Boosters who have made the assertion
that boost::wait(condition, scoped_lock) is a clearer syntactic
expression of the operation's intent.  The image that "waiting" on a
condition brings to my mind is simply that of blocking the thread until
"notification" has occurred from another thread.

Accordingly, I feel that the "wait" operation in this case is not
something that is affecting the condition at all (which is what a member
function implies) but rather something that describes what the calling
thread must do (i.e. wait) before notification to continue arrives (via
condition::notify()).  For this reason I feel that a
boost::wait(condition, scoped_lock) overload should take the place 

* thread::sleep(xtime) --

I do not feel particularly strongly one way or another about the syntax
of this operation because, despite its importance, it is relatively
specialized (as it is not waiting for any completion or notification but
rather only for a certain time to pass).  I do not feel that
thread::sleep(xtime) nor boost::wait(xtime) have any distinguishing
merits independent of other considerations.

If it is decided, however, that boost::wait(...) overloads will be used
to express the functionality for thread and condition waiting, then I
think for the sake of syntactical consistency, there is no reason (other
than the historical one) to discontinue using the "sleep" name in favor
of a boost::wait(xtime) overload.

* scoped_lock::lock() and scoped_lock::unlock() --

I do not see any inconsistency whatsoever in keeping these functions as
members.  If one envisions a lock on a door, it can either be locked or
unlocked; no matter how one looks at a lock (in our case, no matter
which thread looks at it), it is still either locked or unlocked.

This can be contrasted with a condition, which I argued above should
implement its wait operation as a free function.  In contrast with a
lock, a condition might have the real world analogy (sorry if this isn't
perfect) of a horse waiting for the gunshot at a race; each horse at the
race (in our case, each thread) is independent of the other horses
(threads) in whether it is waiting because the only thing each horse
cares about is the gunshot (in our case, the condition notification).
An initial counter-argument to this analogy (i.e., that there is a
centralized gun that has either been shot or not been shot, much like
the lock being locked or unlocked, perhaps) does not stand because a
race track could be implemented equally as well with a gun for each
horse (except for the obvious practical effects of confusing the horses
-- like I said, this isn't exactly the best analogy).  As testimony to
this point is the very fact that both condition::notify() and
condition::notify_all() are clearly defined and implemented.

Thus there are very significant conceptual differences between what goes
on in lock/unlock operations and wait operations; accordingly, I see no
problem in differing syntaxes for each.  Therefore, as I said above, I
do not see any inconsistency whatsoever in keeping these functions as
members while simultaneously making thread and condition wait operations
into free functions.

* semaphore::up() and semaphore::down() --

First of all, I agree with statements made by other Boosters that these
functions should be renamed to semaphore::increment() and
semaphore::decrement() to signify that these functions are indeed
operations to be performed (verbs) rather than simply indicators of
state (as their current adjectival form suggests).  I disagree that they
should be named semaphore::acquire() and semaphore::release() or similar
variants because these names abstract too far, suggesting that somehow a
semaphore can be uniquely held, which is only true in the unique case
that the maximum increment count is set to one; semaphores are
dangerous, and their syntax should not imply otherwise.

As far as continuing the argument over free/member functions, I would
make a very similar argument for semaphore::increment()/decrement()
being kept as member functions as I did above for
scoped_lock::lock()/unlock().  (Of course, my analogy would be
different; the basic point is, however, that much like a lock, a
semaphore conceptually has an independent existence that does not rely
on which thread may happen to view it.)

* Final statement --

Finally, I would like to say that I feel these "free functions" should
be truly free members of the boost namespace rather than static members
of boost::thread or some other class.  My reason for this is that it
facilitates user-defined "wait" operations on user-defined classes via
Koenig lookup.  (This also has the minor, yet positive side effect of
allowing the programmer to omit the boost namespace when calling the
wait function on those types that are defined in the boost library.)

I hope this helps to clear up rather than cloud the discussion.

Thanks,
Eric Friedman