Boost logo

Boost :

From: williamkempf_at_[hidden]
Date: 2001-03-16 09:55:30

--- In boost_at_y..., Jesse Jones <jejones_at_a...> wrote:
> I'm far from a threading expert, but I did take a quick look at the
> and the docs. It looks like a good start, but I do have some
> 1) You should throw an exception if CreateSemaphore returns nil or

Other's have pointed out the lack of error handling. I did this
intentionally in the draft, but it's something that needs addressed
before submission.

> 2) CloseHandle can also fail, but you don't want to throw from a
dtor so an
> assert might be good.

When used properly it's next to impossible for CloseHandle to fail,
and the library insulates the user from misusing CloseHandle. So I
see no need to use an assert here.
> 3) Are there examples where having semaphore::up() return false if
> semaphore is already at the max count is helpful? In general I
don't like
> this sort of thing since return codes are way too easy to ignore in
C. It
> also makes writing the Windows implementation dicier since you
really want
> to distinguish between failing due to max count and due to
something like a
> corrupted object (the ReleaseSemaphore documentation in MSDN doesn't
> provide much guidance in how to do this).

I can't give you specific examples, but this is the behavior one
would expect. Detecting the proper error condition in Win32 should
be trivial.
> 5) To be consistent basic_lock should explicitly privately inherit
> noncopyable.

Ahh... typo. Thanks for catching that one.

> 6) It's better to provide an operator const void*() method instead
> operator bool(). (Unlike bool the const void* one won't implicitly
> to umpteen different types).

This argument has come up on here before, and I'm not 100% sure that
a consensus opinion was reached on this. Conversion to bool is more
natural to my mind, and the implicit conversion "to umpteen different
types" doesn't seem to dangerous to me... at least in this case.
Changing this will be trivial, so I'll defer to the "experts"
opinions on this, but I'm not sure that they've fully agreed with
each other in the past. Am I wrong?

> 7) I don't think the locker classes should themselves throw
exceptions. The
> only way to get this to happen is for the programmer to do
something nasty
> like call unlock too many times. But this is a straight forward
> error, not a runtime error, so it seems like an assert is the

Simple errors can, and will occur:

boost::mutex::lock lock(mutex);
// This code added in maintenance
if (something)
    // ...
// ...
lock.unlock(); // error!

Yes, these are programmer errors, but they are serious errors that
result in exceptional conditions. I think it's appropriate to throw
here, and if you don't throw you'll have to instead return an error.
An assert is not enough.
> 8) It's a little odd that you don't allow clients to call
> basic_lock::lock() twice in a row since the mutex would catch any
> Admittedly this would be a weird thing to do, but the whole point of
> allowing lock is to allow people to do things out of the ordinary,

It just helps to insure that it's less likely a programmer will
misuse the lock by declaring it outside of block scope (like at the
class level). It doesn't fully enforce this, so it may be overkill.
Given the reasoning for it do you (and others) still think it should
not throw in this case?

> 9) I think the code would be clearer if member data names were
> somehow, eg fCount, mCount, _count, etc. :-)

Stylistic suggestion, typically not entertained by Boost
submissions. However, I'm interested in this topic and wouldn't mind
hearing what others have to say on this, even if it's in private e-

Bill Kempf

Boost list run by bdawes at, gregod at, cpdaniel at, john at