[haiku-commits] Re: r38168 - haiku/trunk/src/servers/app

  • From: Stephan Assmus <superstippi@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 18 Aug 2010 08:52:28 +0200

Am 18.08.2010 01:12, schrieb Clemens Zeidler:

Am 18.08.2010, 10:20 Uhr, schrieb Stephan Assmus <superstippi@xxxxxx>:


Am 18.08.2010 00:08, schrieb Stephan Assmus:

For all calls that get called by
ServerWindow threads, the locking needs to work by using the
multi-locker. Basically the data accessed via the Desktop class, and the
data accessed via the ServerWindow class can run in one of three
threads: The desktop thread, the app thread, or the window thread. The
window threads are the performance critical ones, and they should run
concurrently. So whenever something is supposed to run in a ServerWindow
thread only, the read-lock is used from within the ServerWindow threads
themselves. For all other cases, the write lock is used, which makes
sure that no two ServerWindow threads, or ServerApp threads, or the
Desktop thread itself access the same (Desktop owned) data concurrently.
All that being said, I have the impression since you tried the
multi-lock define, and since you added the new message code to the
_MessageNeedsAllWindowsLocked() method, you've had a pretty good idea
about this already, I just wanted to recap a little. :-)


...and yet I am still not happy with the description. Forgot to
mention that this only works, since the code in the Desktop and
ServerApp classes *always* write-lock. If that code would sometimes
read-lock only, it would potentially access the same data that a
ServerWindow thread accesses concurrently. Any data that a
ServerWindow thread accesses with only the read-lock held needs to be
completely private and separate from all other ServerWindows.
Sometimes it's tricky not to introduce deadlocks this way, and I've
considered using the same global MultiLock instance for *every*
locking requirement of the app_server code. Yet there are a few
objects with their own BLocker or even MultiLock, like the globally
shared cursor, various managers like the FontManager, the FontCache
backend, and so on. Most stuff is heavily tested and ok (short
protected "inner" sections of code), but some stuff I am not so sure
about, for example the separate HWInterface MultiLock and how
concurrent access to the same accelerant add-on is handled. At the
moment no acceleration is used at all, but perhaps the locking needs
to be redesigned for it to work properly at all. It's unfortunately
not the freshest thing in my memory either... :-)

Best regards,
-Stephan


Thank you, nice overview! had a rough overview but that makes it even
clearer.

I just read locked it now *) I hope I got it right?

Regards,
Clemens

ps. To access the returned Window you have to hold a lock anyway because
otherwise the Window could have gone in the meantime.

*)
Window*
Desktop::WindowForClientLooperPort(port_id port)
{
AutoReadLocker locker(fWindowLock);

for (Window* window = fAllWindows.FirstWindow(); window != NULL;
window = window->NextWindow(kAllWindowList)) {
if (window->ServerWindow()->ClientLooperPort() == port)
return window;
}
return NULL;
}
Since the window list does not change, a read-lock appears appropriate. However, the code is (probably) broken now, since MultiLocker does not support nested read-locking. If you remove the message code from the list that requires write-locking in ServerWindow, then the code is definitely broken. MultiLocker supports nested write-locks, I don't know what happens when the current write-lock-owner acquires a read-lock. The (read-lock) assert that Axel mentions may indeed be the best solution. 

Best regards,
-Stephan

Other related posts:

  1. Home
  2. haiku-commits
  3. Archive
  4. 08-2010