You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2018/03/15 13:59:08 UTC
Review Request 66088: Fixed crash on macOS during certain
authorization calls.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66088/
-----------------------------------------------------------
Review request for mesos, Benno Evers and Jan Schlicht.
Bugs: MESOS-8677
https://issues.apache.org/jira/browse/MESOS-8677
Repository: mesos
Description
-------
The method in charge of creating `ObjectApprovers` was relying that
iterating over equivalent hashset's would yield the same order of
results.
While that seemed to be true in different architectures, the macOS
implementation did not follow the same pattern showing a bug in the
code.
Diffs
-----
src/common/http.cpp 03f539f8942d50de34f42fd711053e94d0a1c95d
Diff: https://reviews.apache.org/r/66088/diff/1/
Testing
-------
```sh
# In multiple platforms.
make check
```
Thanks,
Alexander Rojas
Re: Review Request 66088: Fixed crash on macOS during certain
authorization calls.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66088/#review199258
-----------------------------------------------------------
Ship it!
src/common/http.cpp
Line 871 (original), 871 (patched)
<https://reviews.apache.org/r/66088/#comment279553>
I'd probably add a comment saying that we have to use `set` here because we relay on the order of the elements being well-defined below. (otherwise the next guy who sees this will probably think "Hey, a `set`, let's change it to `hashset` for a free performance boost" ;)
- Benno Evers
On March 15, 2018, 1:59 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66088/
> -----------------------------------------------------------
>
> (Updated March 15, 2018, 1:59 p.m.)
>
>
> Review request for mesos, Benno Evers and Jan Schlicht.
>
>
> Bugs: MESOS-8677
> https://issues.apache.org/jira/browse/MESOS-8677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method in charge of creating `ObjectApprovers` was relying that
> iterating over equivalent hashset's would yield the same order of
> results.
>
> While that seemed to be true in different architectures, the macOS
> implementation did not follow the same pattern showing a bug in the
> code.
>
>
> Diffs
> -----
>
> src/common/http.cpp 03f539f8942d50de34f42fd711053e94d0a1c95d
>
>
> Diff: https://reviews.apache.org/r/66088/diff/1/
>
>
> Testing
> -------
>
> ```sh
> # In multiple platforms.
> make check
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 66088: Fixed crash on macOS during certain
authorization calls.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On March 15, 2018, 3:36 p.m., Jan Schlicht wrote:
> > src/common/http.cpp
> > Lines 885-886 (original), 885-886 (patched)
> > <https://reviews.apache.org/r/66088/diff/1/?file=1976840#file1976840line885>
> >
> > If it's the ordering of `ObjectApprovers` that's important, then we shouldn't use a `hashmap` for `approvers`.
The order is only relevant for the `map` and `zip` states, we want that the element order match in both operations. Afterwards, it doesn't matter.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66088/#review199259
-----------------------------------------------------------
On March 15, 2018, 3:27 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66088/
> -----------------------------------------------------------
>
> (Updated March 15, 2018, 3:27 p.m.)
>
>
> Review request for mesos, Benno Evers and Jan Schlicht.
>
>
> Bugs: MESOS-8677
> https://issues.apache.org/jira/browse/MESOS-8677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method in charge of creating `ObjectApprovers` was relying that
> iterating over equivalent hashset's would yield the same order of
> results.
>
> While that seemed to be true in different architectures, the macOS
> implementation did not follow the same pattern showing a bug in the
> code.
>
>
> Diffs
> -----
>
> src/common/http.cpp 03f539f8942d50de34f42fd711053e94d0a1c95d
>
>
> Diff: https://reviews.apache.org/r/66088/diff/2/
>
>
> Testing
> -------
>
> ```sh
> # In multiple platforms.
> make check
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 66088: Fixed crash on macOS during certain
authorization calls.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66088/#review199259
-----------------------------------------------------------
src/common/http.cpp
Lines 885-886 (original), 885-886 (patched)
<https://reviews.apache.org/r/66088/#comment279557>
If it's the ordering of `ObjectApprovers` that's important, then we shouldn't use a `hashmap` for `approvers`.
- Jan Schlicht
On March 15, 2018, 3:27 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66088/
> -----------------------------------------------------------
>
> (Updated March 15, 2018, 3:27 p.m.)
>
>
> Review request for mesos, Benno Evers and Jan Schlicht.
>
>
> Bugs: MESOS-8677
> https://issues.apache.org/jira/browse/MESOS-8677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method in charge of creating `ObjectApprovers` was relying that
> iterating over equivalent hashset's would yield the same order of
> results.
>
> While that seemed to be true in different architectures, the macOS
> implementation did not follow the same pattern showing a bug in the
> code.
>
>
> Diffs
> -----
>
> src/common/http.cpp 03f539f8942d50de34f42fd711053e94d0a1c95d
>
>
> Diff: https://reviews.apache.org/r/66088/diff/2/
>
>
> Testing
> -------
>
> ```sh
> # In multiple platforms.
> make check
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 66088: Fixed crash on macOS during certain
authorization calls.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66088/
-----------------------------------------------------------
(Updated March 15, 2018, 3:27 p.m.)
Review request for mesos, Benno Evers and Jan Schlicht.
Bugs: MESOS-8677
https://issues.apache.org/jira/browse/MESOS-8677
Repository: mesos
Description
-------
The method in charge of creating `ObjectApprovers` was relying that
iterating over equivalent hashset's would yield the same order of
results.
While that seemed to be true in different architectures, the macOS
implementation did not follow the same pattern showing a bug in the
code.
Diffs (updated)
-----
src/common/http.cpp 03f539f8942d50de34f42fd711053e94d0a1c95d
Diff: https://reviews.apache.org/r/66088/diff/2/
Changes: https://reviews.apache.org/r/66088/diff/1-2/
Testing
-------
```sh
# In multiple platforms.
make check
```
Thanks,
Alexander Rojas