Susan Hinrichs created TS-3453:
----------------------------------
Summary: 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- 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)
