D22182: Remove anchorClicked lock which causes deadlock

[Aaron Puchert]

aaronpuchert added a comment.


  In D22182#492087 <https://phabricator.kde.org/D22182#492087>, @mswan wrote:
  
  > The workaround being that we eliminate the try-lock behaviour in our locker constructors and move that timeout behaviour elsewhere?
  
  
  Exactly.
  
  >> That's not what we want there, we want an annotation on the function `REQUIRES_SHARED(...)`. Remember that the main purpose of scoped locks is to ensure release on all code paths. If you don't want to release anything, there is no reason for a scoped lock.
  > 
  > Err, so now I'm confused again. So, does `REQUIRES_SHARED(...)` require that **this share** of the lock is no longer held upon completion of this function? If so then is it possible to annotate a function in a semantically identical way to `ENSURE_CAN_READ` which implies that a read (i.e. shared) lock is held before and (possibly) after the function evaluates, or does that kind of behaviour break this whole thing? I imagine then we would have to be much more fine-grained with our usage of DUChain locks and move these lockers closer to the actual mutations to DUChain.
  
  Maybe the terminology is a bit confusing: a "shared" lock is a read lock (many readers can happily coexist), while an "exclusive" lock is a write lock. (The "exclusive" is omitted from the attributes because standard mutexes are always exclusive.) The annotation `REQUIRES_SHARED(m)` means that the function can only be called when `m` is held. Also, `m` should still be held a the end of the function. Basically the attributes encode state transitions:
  
  |                  | unlocked         | locked exclusive             | locked  shared               |
  | unlocked/unknown | `EXCLUDES`       | `ACQUIRE`                    | `ACQUIRE_SHARED`             |
  | locked exclusive | `RELEASE`        | `REQUIRES`                   | `RELEASE` + `ACQUIRE_SHARED` |
  | locked shared    | `RELEASE_SHARED` | `RELEASE_SHARED` + `ACQUIRE` | `REQUIRES_SHARED`            |
  |
  
  where the rows are the state of a mutex at the beginning of a function, and the columns are the state at the end of the function.
  
  So if a function doesn't lock or unlock and just calls `ENSURE_CAN_READ`, it's a hot candidate for `REQUIRES_SHARED`. If we can fix all thread safety warnings and turn them on as errors, we might be able to remove these `ENSURE_*` assertions completely, because we enforce that at compile-time.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D22182

To: mswan, #kdevelop
Cc: aaronpuchert, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190708/d8bb2a98/attachment-0001.html>