[haiku-commits] Re: BRANCH looncraz-github.ScreenStream [b3f82077ee1d] in src: servers/app servers/app/drawing kits/interface

  • From: looncraz <looncraz@xxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 03 Apr 2015 13:01:59 -0500

On 4/3/2015 06:56, Axel Dörfler wrote:

On 04/03/2015 06:16 AM, looncraz-github.ScreenStream wrote:
~AutoReadLocker()
{
- Unlock();
+ while (fLocked > 0)
+ Unlock();
}

That looks wrong. Why have a read locker that locks more than once?


Yes, sorry, that was just for testing to ensure changing to a write lock didn't introduce unintentional nested read locks since I added the ability to regain the lock. I thought about just returning false from Lock() if it was already locked as well, but then we could have unmatched Unlock()s. But, when I used it in practice, that never came up.

Also experimented with a ConvertReaderToWriter() for MultiLocker a year or two ago, but found that a quick mod to AutoReadLocker was cleaner.

+++ b/src/servers/app/Window.cpp
@@ -95,6 +95,8 @@ Window::Window(const BRect& frame, const char *name,
fWindow(window),
fDrawingEngine(drawingEngine),
fDesktop(window->Desktop()),
+ fClippingLock(name),
+


Stray blank line.
Also, this looks pretty wrong to me. The all window lock is pretty much a clipping lock. Why introduce another one to the table?


I did my best to avoid a clipping lock. I previously has ReadLock() and ReadUnlock() added to Window which used the Desktop window lock, but I'd always eventually end up with a deadlock condition somehow. It'd work just fine for a few seconds, sometimes more, then it would just freeze everything.

But, all the added lock contention on the desktop's window lock wasn't such a good thing, either. And it will only get worse as more and more threads become interested in the window's clipping and worse still as client drawing and window clipping become completely separated.

If you have a better idea, I'd absolutely love to hear it. There's a reason I named the commit an experiment ;-)

--The loon


Other related posts: