[interfacekit] Problems in BHandler's watching mechanism
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: "OpenBeOS Interface Kit" <interfacekit@xxxxxxxxxxxxx>
- Date: Sat, 10 Jan 2004 20:14:07 +0100 CET
Hi there,
since I've currently fighting against this issue again in Tracker, here
are some problems we should probably fix/change in our StartWatching()/
SendNotices() implementation/docs (note, I am not that familiar with
our solution):
1) both versions of StartWatching() (handler or messenger) work both
ways - while it probably doesn't make much sense for the BHandler
version, it makes much sense for the BMessenger version to be able to
observe other application's handlers. However, this functionality is
not documented in the BeBook (and should).
(That is, no matter if you call a->StartWatching(b) or vice versa, a->
SendNotices() will notify b, and b->SendNotices() a in both cases)
2) I find the handling of StartWatchingAll()/StopWatchingAll() a bit
strange; unlike its documentation says, StopWatchingAll() does not stop
all monitoring, but only an earlier StartWatchingAll(). So the
following does not work correctly, although it follows the docs:
b->StartWatching(a, '1234');
b->StartWatching(a, '5678');
...
b->StopWatchingAll(a);
3) In addition to the bug described in 2), our handler code is
vulnerable to it as well; it would just crash if the handler "a" is
deleted in the above example. If possible, we should only take the
handler into account for Start/StopWatching(), but SendNotices() should
only use BMessengers in order to prevent us from crashing in that
situation. And since a BMessenger can easily constructed from a
BHandler (and compared as well), and SendNotices() is the most
important function for optimizations, I see no reason to differentiate
between messengers and handlers internally
4) The complete watching mechanism is not thread-safe at all. However,
this makes using it in the documented way overly complicated. I would
suggest adding an extra lock for the observer stuff. Having to lock the
handler's looper might ask for potential deadlocks (and would change
from R5 behaviour too much).
5) I noticed an implementation like this for StopWatching()/
StopWatchingAll() and SendNotices():
void BHandler::SendNotices(uint32 what, const BMessage* msg)
{
fObserverList ? fObserverList : fObserverList = new _ObserverList;
fObserverList->SendNotices(what, msg);
}
I think it only makes sense to create a new ObserverList in
StartWatching() and StartWatchingAll() - what probably could be sent in
SendNotices() or stopped in StopWatching() if there wasn't any list
before?
Also, it makes using SendNotices() without having the looper locked
just too dangerous.
Bye,
Axel.
Other related posts:
- » [interfacekit] Problems in BHandler's watching mechanism
