[haiku-development] invalid BMessengers
- From: Stephan Assmus <superstippi@xxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Wed, 06 Aug 2008 20:59:08 +0200
Hi all,
I could use some advice on a problem I may have found in our BMessenger
implementation. Here is the constructor to save you some time seeking it
out:
BMessenger::BMessenger(const BHandler* handler, const BLooper* looper,
status_t* _result)
:
fPort(-1),
fHandlerToken(B_NULL_TOKEN),
fTeam(-1)
{
status_t error = (handler || looper ? B_OK : B_BAD_VALUE);
if (error == B_OK) {
if (handler) {
// BHandler is given, check/retrieve the looper.
if (looper) {
if (handler->Looper() != looper)
error = B_MISMATCHED_VALUES;
} else {
looper = handler->Looper();
if (looper == NULL)
error = B_MISMATCHED_VALUES;
}
}
// set port, token,...
if (error == B_OK) {
AutoLocker<BLooperList> locker(gLooperList);
if (locker.IsLocked() &&
gLooperList.IsLooperValid(looper)) {
fPort = looper->fMsgPort;
fHandlerToken = (handler
? _get_object_token_(handler) :
B_PREFERRED_TOKEN);
fTeam = looper->Team();
} else
error = B_BAD_VALUE;
}
}
if (_result)
*_result = error;
}
The point of a BMessenger is, among other things, that the user of this
class doesn't need to know if the BHandler and/or BLooper pointers that it
has are still valid at all. At least to a certain point - if only the
handler is known, it needs to be valid of course. If the looper is given
though, the handler should be allowed to be stale though. As you can see
from the above code, BMessenger checks the application global looper list,
which is protected by the "inner" gLooperList lock. It is an inner lock,
which means you are not supposed to grab any additional locks while holding
it, so deadlocks are avoided.
The problem I am seeing in the above code is:
if (looper) {
if (handler->Looper() != looper)
error = B_MISMATCHED_VALUES;
} else {
...
This check means the handler pointer needs to be valid, but I think this is
unintentional. Do you think we could simply remove this check? If the
BHandler does not belong to the BLooper, the error should be detected
later, no? If one would want to check whether or not the BHandler belongs
to the specified looper, without accessing the BHandler object because it
may be stale, one would have to see if the handler is in the looper's
handler list. If I am not missing something, this would require to lock the
looper, which can not be done in the BMessenger constructor, or can it? At
least not while holding the gLooperList lock. The gLooperList lock alone
also can't be enough for this, since although the looper can't go away,
someone else may still be messing with it (like, adding/removing BHandlers).
Any insights?
Best regards,
-Stephan
Other related posts:
