Bug 1647

From Eric Malenfant <EMalenfant@interstarinc.com>:

    TAO VERSION: 1.3.1
    ACE VERSION: 5.3.1

    HOST MACHINE and OPERATING SYSTEM:
        Intel P4, Windows 2000 Pro
        Winsock 2

    TARGET MACHINE and OPERATING SYSTEM, if different from HOST: 
    <same>
    
    COMPILER NAME AND VERSION (AND PATCHLEVEL):
    MSVC 6, SP5

    AREA/CLASS/EXAMPLE AFFECTED:
    ACE_TP_Reactor and TAO_SSLIOP

    DOES THE PROBLEM AFFECT:
        COMPILATION? no
        LINKING? no
        EXECUTION? yes (see description below)

    SYNOPSIS:
ACE_TP_Reactor may end up dispatching same handle on two threads, causing
message corruption in TAO_SSLIOP.

    DESCRIPTION:
First, a little background:

- When ACE_Event_Handler::handle_input() returns > 0, the ACE_Reactor should
  call it again.

- The ACE_TP_Reactor does this with a straightforward "while" loop,
something
  like:
    int status;
    do{
        status = (event_handler->*callback) (handle);
    while (status > 0);

- With the ACE_TP_Reactor, ACE_Event_Handlers can resume their own handle
  before handle_input() returns.

- TAO_SSLIOP_Connection_Handler::handle_input() can return > 0 if its 
  pos_io_hook() detects that there is still data in openssl's buffer.


Now, consider this sequence:

(Thread A) is running the TP_Reactor, and wakes up as data arrives on an 
           SSLIOP connection.
(Thread A) calls TAO_SSLIOP_Connection_Handler, which reads a message, resumes
           the handle, and sees that there is data left in openssl's buffer.
(Thread B) enters the TP_Reactor and wakes up as more data arrived on the same
           SSLIOP connection
(Thread B) starts reading the message.
(Thread A) TAO_SSLIOP_Connection_Handler::handle_input() return 1. As a 
           consequence, TP_Reactor reinvokes it immediately.
-> We now have two threads reading the same handle simultaneously, potentially
   leading to very weird things. In our tests, we saw corrupted messages
   being dispatched to the servant.

    REPEAT BY:
We "discovered" that problem by running a "congestion" test where a 
multithreaded client sends an octet sequence (of approx. 500 bytes) 
and a long to a multithreaded server, using SSLIOP. 
The server sleeps for the time indicated by the second argument (the long).
We rapidly saw that the server was receiving "impossible" values for its
sleep time, and saw that this was because of message corruption on the
server side, because two threads were simultaneously reading on the same
connection.

    SAMPLE FIX/WORKAROUND:
Assuming that ACE_Event_Handler::handle_input() returning > 0 does not mean
"Call me again immediately", but more something like "Leave my handle in the

ready set so that I will be called again next time someone wants to dispatch
an event", we tought about doing just that, i.e., putting the Handler's handle
in the Reactor's ready set.

However, the TP_Reactor flushes the ready set when its "state_changed_"
member is set, as a result of "register/remove_handler()". Also, we felt that
this may cause fairness problems. So we expanded the idea a bit and added
another handle set to the TP_Reactor (called the "scheduled_set"). Handles of
Event_Handlers returning > 0 are put in that scheduled_set, and the 
TP_Reactor's dispatching loop was modified so that, when the ready_set is
empty, instead of immediately going to wait into select(), we:
- Look in the scheduled_set
- If it is not empty, we call select() with a zero timeout (so that other
  handles have a chance to have their events processed even if another
  handle stays a long time in the "scheduled_set")
- Merge the result from select() and the contents of the scheduled_set
  into the ready_set
- Dispatch events from the ready_set, removing the handles from the ready_set
  and the scheduled_set as they are dispatched.