-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69123/#review210024
-----------------------------------------------------------
Not yours, I see in `MemorySubsystemProcess::oomListen` we have the these code:
```
info->oomNotifier.onReady(
defer(PID<MemorySubsystemProcess>(this),
&MemorySubsystemProcess::oomWaited,
containerId,
cgroup,
lambda::_1));
```
Here the `onReady` seems not correct to me, I think it should be `onAny`.
src/linux/cgroups.cpp
Lines 1071-1080 (original), 1071-1089 (patched)
<https://reviews.apache.org/r/69123/#comment294661>
I see we already have an onAny callback `_listen`, can we close the fd at
the end of that method (i.e., when we are sure that we will not listen
anymore)? And then we could change the code here like below:
```
if (reading.isSome()) {
reading->discard();
} else if (eventfd.isSome()) {
Try<Nothing> unregister = unregisterNotifier(eventfd.get());
if (unregister.isError()) {
LOG(ERROR) << "Failed to unregister eventfd: " << unregister.error();
}
}
```
- Qian Zhang
On Oct. 23, 2018, 11:08 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69123/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2018, 11:08 a.m.)
>
>
> Review request for mesos and Qian Zhang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The cgroups event notifier was closing the eventfd while an
> `io::read()` operation may be in progress. This can lead to
> bugs where the fd gets re-used and read from a stale io::read.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp 95efc1def3318dd93d30d91e9c8a91376cc610b8
>
>
> Diff: https://reviews.apache.org/r/69123/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
