AdaCore / AdaCore/gnatcoll-core
Race condition in Weak References implementation
Nobody has claimed this yet.
- Dominant language
- Ada
- Stars
- 55
- Forks
- 36
- PR merge metrics
- No merged PRs in 30d
Description
There is a possible race condition in between Proxied read and test for reference counter = 0 in Proxied. A thread resolving weak reference could have read non-null Proxied into register and preemted, and after resume it will access invalid memory. Proxied is null on resume, but this check is passed.
The only good protection against it that comes to my mind is to introduce integer counter in proxy and increment it when reading it. This counter must be strongly associated with Proxied and modified atomically together with it using cmgxchg8b, cmpxchg16b or mutex.
Pseudocode for resolving weak reference:
declare
Saved_Proxied : Proxy_Access;
Saved_Lock : Atomic_Unsigned;
begin
loop
Saved_Proxied := Proxy.Proxied;
if Saved_Proxied = null then
return Null_Reference;
end if;
Saved_Lock := Proxy.Lock;
exit when Proxy.Compare_And_Swap
(Proxy.Proxied, Proxy.Lock,
Saved_Proxied, Saved_Lock,
Saved_Proxied, Saved_Lock + 1);
end loop;
if Saved_Proxied.Reference_Counter = 0 or Saved_Proxied.Is_Beeing_Freed then
loop
Saved_Proxied := Proxy.Proxied;
Saved_Lock := Proxy.Lock;
exit when Proxy.Compare_And_Swap
(Proxy.Proxied, Proxy.Lock,
Saved_Proxied, Saved_Lock,
Saved_Proxied, Saved_Lock - 1);
end loop;
return Null_Reference;
end if;
return Result : Reference := Create_Reference (Saved_Proxied) do
loop
Saved_Proxied := Proxy.Proxied;
if Saved_Proxied = null then
Result := Null_Reference;
loop
Saved_Proxied := Proxy.Proxied;
Saved_Lock := Proxy.Lock;
exit when Proxy.Compare_And_Swap
(Proxy.Proxied, Proxy.Lock,
Saved_Proxied, Saved_Lock,
Saved_Proxied, Saved_Lock - 1);
end loop;
return;
end if;
Saved_Lock := Proxy.Lock;
exit when Proxy.Compare_And_Swap
(Proxy.Proxied, Proxy.Lock,
Saved_Proxied, Saved_Lock,
Saved_Proxied, Saved_Lock - 1);
end loop;
end return;
end;
Pseudocode for zeroing Proxied:
declare
Saved_Proxied : Proxy_Access;
Saved_Lock : Atomic_Unsigned;
begin
loop
Saved_Proxied := Proxy.Proxied;
exit when Saved_Proxied = null;
Saved_Lock := Proxy.Lock;
exit when
Proxy.Compare_And_Swap
(Proxy.Proxied, Proxy.Lock,
Saved_Proxied, Saved_Lock,
null, Saved_Lock);
end loop;
while Saved_Lock /= 0 loop
Saved_Lock := Proxy.Lock;
end loop;
end;
This way reading Proxied becomes atomic. Reader thread technically reads value (Saved_Proxied := Proxy.Proxied), but cannot legally do anything useful with it, even dereference, until it puts a lock. Thread that is going to destroy object, first zeroes Proxy.Proxied, so that new threads can no longer appear, then waits for every remaining reader thread to abort. Then it can safely deallocate memory.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the Weak References implementation and the Proxied read, reference-counter, and destruction paths mentioned in the report. Review the existing concurrency tests, if any, and establish a reproducible reader-versus-destruction case. Done means weak-reference resolution cannot access freed memory; the issue names no repository files or tests.
Written by the indexing model from the issue text.
Assessment
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100