127740 - Thread yield in ssl3_SendApplicationData

Status: RESOLVED INVALID

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

I found this problem during code inspection.

The ssl3_SendApplicationData function in ssl3con.c
does a thread yield (PR_Sleep with zero timeout)
between the ssl3_SendRecord calls.  This thread
yield should not be necessary, at least the reason
is not documented in the comment.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

In what sense is this a "problem"?

libSSL supports simultaneous two-way exchange of application data once
the initial handshake is done.  This is (er, may be) accomplished using
two threads, one to do the reading on the SSL socket, and one to do the 
writing.  libSSL explicitly allows this, which is why there are separate
locks at each level for the "read side" and "write side" of SSL.

It is possible for the server to initiate a second handshake while the 
writer thread is in the middle of a large application write.  In that
case, the reader thread will receive the server's request to conduct
another handshake.  It is important that the handshake be conducted
as quickly as possible.  More to the point, if the writer thread was,
say, writing a megabyte or two, and after the first 1k bytes, the server
requested another handshake, you would not want the handshake to be held
off until the entire megabyte was written.  This is why the xmit buf
lock is released and reacquired each time (after the first) through the 
loop that sends application data.  

The sleep is intended to give the reader thread a chance to get some
cycles in there.  Without this sleep, on a single CPU box, I found that
other thread sometimes just didn't get a chance to run until the writer
thread had gone through this loop many times.  

Comment 2

[Wan-Teh Chang]

Thanks for the explanation, Nelson.  I should have called this
a *potential* problem.  In general, I am worried about thread
yields.  Thread yields may potentially have two problems.

1. Some people use thread yield to do what should have been
done with a condition variable.  It's obvious that this is
not the case here.

2. In some threading API, a thread yield only yields to
runnable threads that have a higher priority.  So I wanted
to make sure that the thread yield in ssl3_SendApplicationData
is just a performance optimization and we are not relying on
it for correctness.

I also thought it would be nice to have a comment that explains
the purpose of that thread yield call.

Comment 3

[Wan-Teh Chang]

Changed the QA contact to Bishakha.

Comment 4

[Wan-Teh Chang]

I will add a comment and mark this bug invalid.

Comment 5

[Wan-Teh Chang]

Added a comment to explain the purpose of the thread
yield in ssl3_SendApplicationData.  Marked the bug
INVALID.