Re: [PATCH v6 6/6] rust_binder: use bitmap for allocation of handles

[Burak Emir]

> > +
> > +            let grow_request = refs.handle_is_present.grow_request().ok_or(ENOMEM)?;
> > +            drop(refs_lock);
> > +            let resizer = grow_request.realloc(GFP_KERNEL)?;
> > +            refs_lock = self.node_refs.lock();
> > +            refs = &mut *refs_lock;
> > +            refs.handle_is_present.grow(resizer);
>
> This continues puzzling me. Refs_lock protects refs, and the spec
> says:
>
>     a reference’s scope starts from where it is introduced and
>     continues through the last time that reference is used.
>
> https://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html
>
> The last usage of refs is at .grow_request() line, because later it's
> reused with the new value.
>
> If my reading of the spec is correct, after dropping the refs_lock,
> you may get rescheduled, and another thread may follow the same path.
> Because refs_lock is dropped explicitly and refs - implicitly, the
> concurrent thread can grab both and follow with resizing the id map.
>
> When your first thread will get back, you'll end up resizing the
> already resized map.
>
> I asked your AI, and it says that this race is indeed possible for
> exactly that reason. But it doesn't break memory safety, so the
> compiler is happy about it...

Yes - the writes to the map field are synchronized.
So there is no data race (= unsynchronized {read, write} / write
conflict) and corruption cannot happen.
Never mind that there is a race involving data : )

The scenario you describe is why id_pool grow() and shrink()
double-check before doing copying.
So the thread that loses the race has allocated a new array for
nothing, but things move on.