[
https://issues.apache.org/jira/browse/TS-3453?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14370117#comment-14370117
]
Susan Hinrichs commented on TS-3453:
------------------------------------
Not yet fixed. Not addressing a specific error, so waiting for feedback from
others who came before.
> Confusion of handling SSL events in write_to_net_io in UnixNetVConnection.cc
> ----------------------------------------------------------------------------
>
> Key: TS-3453
> URL: https://issues.apache.org/jira/browse/TS-3453
> Project: Traffic Server
> Issue Type: Bug
> Components: SSL
> Reporter: Susan Hinrichs
>
> While tracking down differences for SSL between 5.0 and 5.2 for TS-3451 I
> came across odd event handling code in write_to_net_io in
> UnixNetVConnection.cc. Looking back at the history in that code things
> became even more confusing.
> The current version on master (same as what is in 5.2) contains the following
> in an if/else sequence. SSL IOs can return READ events event from write
> functions (and visa versa), so that is why I assume that this write function
> is dealing with SSL read events at all.
> {code}
> if (ret == SSL_HANDSHAKE_WANT_READ || ret == SSL_HANDSHAKE_WANT_ACCEPT ||
> ret == SSL_HANDSHAKE_WANT_CONNECT
> || ret == SSL_HANDSHAKE_WANT_WRITE) {
> vc->read.triggered = 0;
> nh->read_ready_list.remove(vc);
> vc->write.triggered = 0;
> nh->write_ready_list.remove(vc);
> if (ret == SSL_HANDSHAKE_WANT_READ || ret == SSL_HANDSHAKE_WANT_ACCEPT)
> read_reschedule(nh, vc);
> else
> write_reschedule(nh, vc);
> }
> {code}
> Seems odd to be clearing and remove read events if the real event was only a
> write. And visa versa, seems odd to be clearing and removing write events if
> the real event was a read.
> It seems to me that the sequence should be replaced with something like
> {code}
> if (ret == SSL_HANDSHAKE_WANT_READ || ret == SSL_HANDSHAKE_WANT_ACCEPT) {
> vc->read.triggered = 0;
> nh->read_ready_list.remove(vc);
> read_reschedule(nh, vc);
> } else if (ret == SSL_HANDSHAKE_WANT_CONNECT || ret ==
> SSL_HANDSHAKE_WANT_WRITE) {
> vc->write.triggered = 0;
> nh->write_ready_list.remove(vc);
> write_reschedule(nh, vc);
> }
> {code}
> Looking back at the history shows adding and removing and re-adding of
> reschedules.
> * TS-3006 9/22/14 by me. Adds in the read_reschedule case
> * TS-2815 5/16/14 by [~bcall] Removes the read_reschedule case
> * TS-2211 10/28/13 by postwait Adds read_reschedule and protects the
> write_reschedule and read_reschedule with specific event checks.
> * TS-1921 5/17/13 by [~jpe...@apache.org] Adds in the write_reschedule
> This seems like an obvious tidy up thing. I'm not addressing a specific
> issue here, but the current thing seems wrong. Given the history, I'm
> hesitant to clean things up without review from those that came before.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
