You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2018/06/11 12:36:47 UTC
Review Request 67525: Removed static variable with non-constant
initialization.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos
Description
-------
The static variable `http::statuses` was initialized using a
dynamic memory allocation. This is against our coding style,
and makes it as easy as writing
static process::http::NotFound DEFAULT_RESPONSE;
for a user to introduce a potential segfault into his code.
Therefore, it was changed to use an array with constant
initialization instead.
The pointer `statuses` was also removed from the public API, since
there are no current users outside libprocess, providing a non-const
pointer to an internal data structure seems inherently unsafe, and
the intended use-cases seem to be covered by the existing `Status::string()`
function.
Diffs
-----
3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
Diff: https://reviews.apache.org/r/67525/diff/1/
Testing
-------
`make check`
Thanks,
Benno Evers
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204562
-----------------------------------------------------------
Bad review!
Reviews applied: []
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On June 11, 2018, 12:36 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 11, 2018, 12:36 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
> Therefore, it was changed to use an array with constant
> initialization instead.
>
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
>
>
> Diff: https://reviews.apache.org/r/67525/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204683
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['67525']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67525
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67525/logs/mesos-tests-stdout.log):
```
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (127 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (1131 ms total)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (36 ms)
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (40 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (80 ms total)
[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[ OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (851 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (875 ms total)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (843 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (867 ms total)
[----------] Global test environment tear-down
[==========] 988 tests from 97 test cases ran. (496495 ms total)
[ PASSED ] 987 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] MesosContainerizer/DefaultExecutorTest.SigkillExecutor/0, where GetParam() = "mesos"
1 FAILED TEST
YOU HAVE 220 DISABLED TESTS
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67525/logs/mesos-tests-stderr.log):
```
I0613 11:51:07.106554 716 slave.cpp:3939] Shutting down framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000
I0613 11:51:07.106554 1328 master.cpp:10863] Updating the state of task 9114be25-ec5c-4382-825a-06f69637a116 of framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0613 11:51:07.106554 716 slave.cpp:6660] Shutting down executor '9114be25-ec5c-4382-825a-06f69637a116' of framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000 at executor(1)@192.10.1.5:52339
I0613 11:51:07.108556 716 slave.cpp:931] Agent terminating
W0613 11:51:07.109555 716 slave.cppI0613 11:51:06.933574 604 exec.cpp:162] Version: 1.7.0
I0613 11:51:06.961549 2352 exec.cpp:236] Executor registered on agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0
I0613 11:51:06.965580 2124 executor.cpp:178] Received SUBSCRIBED event
I0613 11:51:06.970556 2124 executor.cpp:182] Subscribed executor on windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net
I0613 11:51:06.971556 2124 executor.cpp:178] Received LAUNCH event
I0613 11:51:06.976554 2124 executor.cpp:665] Starting task 9114be25-ec5c-4382-825a-06f69637a116
I0613 11:51:07.062561 2124 executor.cpp:485] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I0613 11:51:07.076561 2124 executor.cpp:678] Forked command at 7056
I0613 11:51:07.109555 7648 exec.cpp:445] Executor asked to shutdown
I0613 11:51:07.110556 2124 executor.cpp:178] Received SHUTDOWN event
I0613 11:51:07.110556 2124 executor.cpp:781] Shutting down
I0613 11:51:07.110556 2124 executor.cpp:894] Sending SIGTERM to process tree at pid 705:3935] Ignoring shutdown framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000 because it is terminating
I0613 11:51:07.110556 1328 master.cpp:10962] Removing task 9114be25-ec5c-4382-825a-06f69637a116 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000 on agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 (windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613 11:51:07.113554 5536 master.cpp:1293] Agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 (windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net) disconnected
I0613 11:51:07.113554 5536 master.cpp:3303] Disconnecting agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 (windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613 11:51:07.114547 5536 master.cpp:3322] Deactivating agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 at slave(449)@192.10.1.5:52318 (windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0613 11:51:07.114547 6332 hierarchical.cpp:344] Removed framework 91665eeb-576a-4d3e-94f9-647cc079b9d8-0000
I0613 11:51:07.115566 5316 hierarchical.cpp:766] Agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0 deactivated
I0613 11:51:07.116557 5536 containerizer.cpp:2405] Destroying container aeb5116f-b05f-4804-9723-5a197177642c in RUNNING state
I0613 11:51:07.116557 5536 containerizer.cpp:3019] Transitioning the state of container aeb5116f-b05f-4804-9723-5a197177642c from RUNNING to DESTROYING
I0613 11:51:07.116557 5536 launcher.cpp:155] Asked to destroy container aeb5116f-b05f-4804-9723-5a197177642c
I0613 11:51:07.207556 5316 containerizer.cpp:2858] Container aeb5116f-b05f-4804-9723-5a197177642c has exited
I0613 11:51:07.239590 6888 master.cpp:1135] Master terminating
I0613 11:51:07.242573 8152 hierarchical.cpp:609] Removed agent 91665eeb-576a-4d3e-94f9-647cc079b9d8-S0
I0613 11:51:07.575206 1140 process.cpp:940] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On June 13, 2018, 2:23 a.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 13, 2018, 2:23 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable was initialized using a dynamic memory
> allocation. This is against our coding style, and makes it
> as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
> 3rdparty/libprocess/src/tests/http_tests.cpp ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d
>
>
> Diff: https://reviews.apache.org/r/67525/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204722
-----------------------------------------------------------
Bad review!
Reviews applied: []
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On June 13, 2018, 9:23 a.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 13, 2018, 9:23 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable was initialized using a dynamic memory
> allocation. This is against our coding style, and makes it
> as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
> 3rdparty/libprocess/src/tests/http_tests.cpp ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d
>
>
> Diff: https://reviews.apache.org/r/67525/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204779
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On June 13, 2018, 9:23 a.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 13, 2018, 9:23 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable was initialized using a dynamic memory
> allocation. This is against our coding style, and makes it
> as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
> 3rdparty/libprocess/src/tests/http_tests.cpp ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d
>
>
> Diff: https://reviews.apache.org/r/67525/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/
-----------------------------------------------------------
(Updated June 13, 2018, 9:23 a.m.)
Review request for mesos.
Changes
-------
Added tests.
Repository: mesos
Description (updated)
-------
The static variable was initialized using a dynamic memory
allocation. This is against our coding style, and makes it
as easy as writing
static process::http::NotFound DEFAULT_RESPONSE;
for a user to introduce a potential segfault into his code.
Diffs (updated)
-----
3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
3rdparty/libprocess/src/tests/http_tests.cpp ca1ea116866b5ccd3d42e93dd5bd7fed2234df0d
Diff: https://reviews.apache.org/r/67525/diff/2/
Changes: https://reviews.apache.org/r/67525/diff/1-2/
Testing
-------
`make check`
Thanks,
Benno Evers
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204538
-----------------------------------------------------------
3rdparty/libprocess/include/process/http.hpp
Line 121 (original)
<https://reviews.apache.org/r/67525/#comment287095>
I put this in the review, but actually I'm not sure if this is ok regarding our rules concerning API and ABI stability.
If desired, I can change to one of these alternatives:
1) Keep the hashmap in the public API, mark it as deprecated
2) Remove hashmap from the public API, but still define it in `http.cpp` to keep ABI compatibility;
- Benno Evers
On June 11, 2018, 12:36 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 11, 2018, 12:36 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
> Therefore, it was changed to use an array with constant
> initialization instead.
>
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
>
>
> Diff: https://reviews.apache.org/r/67525/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 67525: Removed static variable with non-constant
initialization.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67525/#review204540
-----------------------------------------------------------
PASS: Mesos patch 67525 was successfully built and tested.
Reviews applied: `['67525']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67525
- Mesos Reviewbot Windows
On June 11, 2018, 12:36 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> -----------------------------------------------------------
>
> (Updated June 11, 2018, 12:36 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
>
> static process::http::NotFound DEFAULT_RESPONSE;
>
> for a user to introduce a potential segfault into his code.
>
> Therefore, it was changed to use an array with constant
> initialization instead.
>
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/http.hpp 055447e13117c0a3ba79d0fc326ece657e8f064f
> 3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658
> 3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501
>
>
> Diff: https://reviews.apache.org/r/67525/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benno Evers
>
>