Skip to content

[DRAFT] [starvation] match locks reached through members with the locks of their class - #2205

Closed
VladimirMakaev wants to merge 2 commits into
meta:mainfrom
VladimirMakaev:starvation-member-locks
Closed

VladimirMakaev wants to merge 2 commits into
meta:mainfrom
VladimirMakaev:starvation-member-locks

Conversation

@VladimirMakaev

@VladimirMakaev VladimirMakaev commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Stack: #2167 → #2205 (review only the top commit; lands after #2167)

Starvation matched locks across threads only with equal root types and accesses, and searched for
the other thread only in the root class of a lock, so Client's this->owner->first never matched
Owner's this->first:

#include <mutex>

struct Owner {
  std::mutex first, second;
  void lock_first_then_second() { std::lock_guard<std::mutex> l1(first), l2(second); }
};
struct Client {
  Owner* owner;
  void lock_second_then_first() {
    std::lock_guard<std::mutex> l1(owner->second), l2(owner->first); // Infer missed: DEADLOCK
  }
};

An object reached through fields may now be the root of another thread's lock, unless an earlier
object on the path has its class (as in a list), and every class on the lock paths is searched,
also in Java and for C structs in whole-program mode. Deduplication keeps deadlocks that only one
side finds. A thread in wait() blocks the UI thread only if another lock it holds may be the UI
thread's.

Limitations: gate locks reached through members count as common, as for parameters; starvation is
searched only from the UI thread, which does not match its this->m with another class's
this->owner->m.

Test plan

New tests in member_locks.cpp, member_locks.c, MemberLocks.java and ObjWait.java, and new
cpp/starvation-dedup and c/starvation-dedup directories. Main reports only the TreeNode
deadlocks, FN_gate_then_ba_bad and ObjWaitHoldsOtherParam, and falsely ObjWaitOnOnlyLock.
Parameters.oneWaySyncOnParamBad now reports the other side of its deadlock. All codetoanalyze
tests pass.

…only in qualifiers

The clang frontend types `this` as `C const *` in the const methods of `C` and as `C *` elsewhere.
RacerD and Starvation compared these types with their qualifiers when pairing accesses or locks of
two methods, missing races and deadlocks involving const methods:

```cpp
#include <mutex>

class C {
  std::mutex m_;
  int x_;

 public:
  int get() const { return x_; } // Infer missed: LOCK_CONSISTENCY_VIOLATION
  void set(int v) {
    std::lock_guard<std::mutex> l(m_);
    x_ = v;
  }
};
```

RacerD now ignores the base type when grouping accesses, as `AccessPath.compare` does. Starvation
compares the root types of parameter-rooted locks across threads modulo qualifiers, so `const C *`
parameters also pair with `C *` ones.

Trade-off: two-instance deadlocks with const on one side only, such as `x.copy_from(&y)` against
`y.copy_to(&x)`, are no longer reported, like the same code without const.

## Test plan

New tests `cpp/racerd/const_methods.cpp` and `cpp/starvation/const_methods.cpp`. Of their `_bad`
cases, main reports only the two-instance deadlock `FN_copy_from_bad`, which is no longer
reported. All codetoanalyze tests pass.
…eir class

Starvation matched locks across threads only with equal root types and accesses, and searched for
the other thread only in the root class of a lock, so `Client`'s `this->owner->first` never matched
`Owner`'s `this->first`:

```cpp
#include <mutex>

struct Owner {
  std::mutex first, second;
  void lock_first_then_second() { std::lock_guard<std::mutex> l1(first), l2(second); }
};
struct Client {
  Owner* owner;
  void lock_second_then_first() {
    std::lock_guard<std::mutex> l1(owner->second), l2(owner->first); // Infer missed: DEADLOCK
  }
};
```

An object reached through fields may now be the root of another thread's lock, unless an earlier
object on the path has its class (as in a list), and every class on the lock paths is searched,
also in Java and for C structs in whole-program mode. Deduplication keeps deadlocks that only one
side finds. A thread in `wait()` blocks the UI thread only if another lock it holds may be the UI
thread's.

Limitations: gate locks reached through members count as common, as for parameters; starvation is
searched only from the UI thread, which does not match its `this->m` with another class's
`this->owner->m`.

## Test plan

New tests in `member_locks.cpp`, `member_locks.c`, `MemberLocks.java` and `ObjWait.java`, and new
`cpp/starvation-dedup` and `c/starvation-dedup` directories. Main reports only the `TreeNode`
deadlocks, `FN_gate_then_ba_bad` and `ObjWaitHoldsOtherParam`, and falsely `ObjWaitOnOnlyLock`.
`Parameters.oneWaySyncOnParamBad` now reports the other side of its deadlock. All codetoanalyze
tests pass.
@meta-cla meta-cla Bot added the CLA Signed label Oct 3, 2026
@VladimirMakaev
VladimirMakaev force-pushed the starvation-member-locks branch 2 times, most recently from 6fc91de to 8116f88 Compare October 5, 2026 06:55
VladimirMakaev added a commit to VladimirMakaev/infer that referenced this pull request Oct 5, 2026
…eir class

Starvation matched locks across threads only with equal root types and accesses, and searched for
the other thread only in the root class of a lock, so `Client`'s `this->owner->first` never matched
`Owner`'s `this->first`:

Upstream-PR: meta#2205
PR-Head: 8116f88
PR-Base: 8ecc034
@VladimirMakaev
VladimirMakaev force-pushed the starvation-member-locks branch from 8116f88 to 60c0c16 Compare October 7, 2026 15:56
@VladimirMakaev

Copy link
Copy Markdown
Contributor Author

Will reopen once current PR queue is cleared

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant