You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/09/02 00:38:05 UTC

Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/
-----------------------------------------------------------

Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.


Repository: mesos


Description
-------

This updates the comments to include my findings after having looked
into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
write path errors separately. Unfortunately, we cannot do this using
libevent 2.0.x (see the comments).

This also explains the "dirty" SSL shutdown case.


Diffs
-----

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 


Diff: https://reviews.apache.org/r/62051/diff/1/


Testing
-------

N/A


Thanks,

Benjamin Mahler


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/#review184406
-----------------------------------------------------------



Patch looks great!

Reviews applied: [62049, 62050, 62051]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Sept. 2, 2017, 8:38 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62051/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2017, 8:38 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the comments to include my findings after having looked
> into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
> write path errors separately. Unfortunately, we cannot do this using
> libevent 2.0.x (see the comments).
> 
> This also explains the "dirty" SSL shutdown case.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 
> 
> 
> Diff: https://reviews.apache.org/r/62051/diff/1/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/#review184415
-----------------------------------------------------------



Patch looks great!

Reviews applied: [62049, 62050, 62051]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Sept. 2, 2017, 3:16 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62051/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2017, 3:16 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the comments to include my findings after having looked
> into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
> write path errors separately. Unfortunately, we cannot do this using
> libevent 2.0.x (see the comments).
> 
> This also clarifies the "dirty" SSL shutdown case and the case where
> futher sends are performed on a shut down socket.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 
> 
> 
> Diff: https://reviews.apache.org/r/62051/diff/2/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/
-----------------------------------------------------------

(Updated Sept. 2, 2017, 3:16 a.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.


Changes
-------

Documented another case based on jie's analysis.


Repository: mesos


Description (updated)
-------

This updates the comments to include my findings after having looked
into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
write path errors separately. Unfortunately, we cannot do this using
libevent 2.0.x (see the comments).

This also clarifies the "dirty" SSL shutdown case and the case where
futher sends are performed on a shut down socket.


Diffs (updated)
-----

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 


Diff: https://reviews.apache.org/r/62051/diff/2/

Changes: https://reviews.apache.org/r/62051/diff/1-2/


Testing
-------

N/A


Thanks,

Benjamin Mahler


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/#review184409
-----------------------------------------------------------


Ship it!




Ship It!

- Chun-Hung Hsiao


On Sept. 2, 2017, 12:38 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62051/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the comments to include my findings after having looked
> into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
> write path errors separately. Unfortunately, we cannot do this using
> libevent 2.0.x (see the comments).
> 
> This also explains the "dirty" SSL shutdown case.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 
> 
> 
> Diff: https://reviews.apache.org/r/62051/diff/1/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Sept. 2, 2017, 3:09 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp
> > Line 365 (original), 380 (patched)
> > <https://reviews.apache.org/r/62051/diff/1/?file=1813748#file1813748line389>
> >
> >     it's also possible that SSL session has already been shutdown. Any subsequent `send` call will trigger `BEV_EVENT_ERROR` with `errno=0`.

Good find, updated to reflect this case.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/#review184412
-----------------------------------------------------------


On Sept. 2, 2017, 12:38 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62051/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the comments to include my findings after having looked
> into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
> write path errors separately. Unfortunately, we cannot do this using
> libevent 2.0.x (see the comments).
> 
> This also explains the "dirty" SSL shutdown case.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 
> 
> 
> Diff: https://reviews.apache.org/r/62051/diff/1/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 62051: Clarified some issues in the LibeventSSLSocket event_callback logic.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62051/#review184412
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/libprocess/src/libevent_ssl_socket.cpp
Line 365 (original), 380 (patched)
<https://reviews.apache.org/r/62051/#comment260598>

    it's also possible that SSL session has already been shutdown. Any subsequent `send` call will trigger `BEV_EVENT_ERROR` with `errno=0`.


- Jie Yu


On Sept. 2, 2017, 12:38 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62051/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This updates the comments to include my findings after having looked
> into using BEV_EVENT_READING / BEV_EVENT_WRITING to handle read and
> write path errors separately. Unfortunately, we cannot do this using
> libevent 2.0.x (see the comments).
> 
> This also explains the "dirty" SSL shutdown case.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 0fa7565d1dc49e53890708df154c4d8ea278ad37 
> 
> 
> Diff: https://reviews.apache.org/r/62051/diff/1/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>