You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/01/11 02:19:11 UTC
Re: Review Request 55024: Windows: Start the socket stack in
`process::initialize`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55024/#review161171
-----------------------------------------------------------
3rdparty/libprocess/src/process.cpp (lines 1040 - 1041)
<https://reviews.apache.org/r/55024/#comment232406>
Even if this is idempotent, we can remove the call from some of our own binaries now.
Also, what is the version check you mention here?
3rdparty/libprocess/src/process.cpp (line 1046)
<https://reviews.apache.org/r/55024/#comment232407>
s/teard down/teardown/
`process::finalize` should probably perform the socket teardown, or at least give the option to do so.
Currently, very few of our processes call `process::finalize`. They usually just rely on the OS cleaning up after them.
- Joseph Wu
On Dec. 24, 2016, 2:46 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55024/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2016, 2:46 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently libprocess will attempt to use sockets without initializing
> the socket stack on Windows. This commit will resolving this problem by
> causing `process::initialize` to initialize the socket stack.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 889a03444eaee7b5ad2be65bb414c30062d4a4f0
>
> Diff: https://reviews.apache.org/r/55024/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55024: Windows: Start the socket stack in
`process::initialize`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Jan. 11, 2017, 2:19 a.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1046
> > <https://reviews.apache.org/r/55024/diff/1/?file=1591723#file1591723line1046>
> >
> > s/teard down/teardown/
> >
> > `process::finalize` should probably perform the socket teardown, or at least give the option to do so.
> >
> > Currently, very few of our processes call `process::finalize`. They usually just rely on the OS cleaning up after them.
I'm pretty comfortable allowing `process::finalize` to clean up after itself, especially resources that it owns and abstracts away, but disposing of something that libprocess _doesn't_ own, and especially something as global and critical as the socket stack, seems a bit out of scope.
I do think it's prudent to entertain proposals about having it _optionally_ dispose of the socket stack, but I can't think of an obviously good way to do this. A default parameter in `process::finalize` would only be meaningful semantically on Windows, no?
> On Jan. 11, 2017, 2:19 a.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 1040-1041
> > <https://reviews.apache.org/r/55024/diff/1/?file=1591723#file1591723line1040>
> >
> > Even if this is idempotent, we can remove the call from some of our own binaries now.
> >
> > Also, what is the version check you mention here?
I'll clarify the comment. What I was saying is that `wsa_initialize` will turn on the socket stack (which may be only on, which is fine because it's idempotent), but also check that its version is compatible with Stout and libprocess.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55024/#review161171
-----------------------------------------------------------
On Dec. 24, 2016, 10:46 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55024/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2016, 10:46 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently libprocess will attempt to use sockets without initializing
> the socket stack on Windows. This commit will resolving this problem by
> causing `process::initialize` to initialize the socket stack.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 889a03444eaee7b5ad2be65bb414c30062d4a4f0
>
> Diff: https://reviews.apache.org/r/55024/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55024: Windows: Start the socket stack in
`process::initialize`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Jan. 11, 2017, 2:19 a.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1046
> > <https://reviews.apache.org/r/55024/diff/1/?file=1591723#file1591723line1046>
> >
> > s/teard down/teardown/
> >
> > `process::finalize` should probably perform the socket teardown, or at least give the option to do so.
> >
> > Currently, very few of our processes call `process::finalize`. They usually just rely on the OS cleaning up after them.
>
> Alex Clemmer wrote:
> I'm pretty comfortable allowing `process::finalize` to clean up after itself, especially resources that it owns and abstracts away, but disposing of something that libprocess _doesn't_ own, and especially something as global and critical as the socket stack, seems a bit out of scope.
>
> I do think it's prudent to entertain proposals about having it _optionally_ dispose of the socket stack, but I can't think of an obviously good way to do this. A default parameter in `process::finalize` would only be meaningful semantically on Windows, no?
Per Slack conversation, we've decided to add an optional parameter to perform WSA cleanup.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55024/#review161171
-----------------------------------------------------------
On Dec. 24, 2016, 10:46 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55024/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2016, 10:46 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Currently libprocess will attempt to use sockets without initializing
> the socket stack on Windows. This commit will resolving this problem by
> causing `process::initialize` to initialize the socket stack.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 889a03444eaee7b5ad2be65bb414c30062d4a4f0
>
> Diff: https://reviews.apache.org/r/55024/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>