> On June 29, 2016, 9:02 p.m., anilkumar gingade wrote: > > Nice to have unit test for this.
Agreed, however this would require quite a few test hooks in product code. There are multiple scenarios that can cause this issue. I've inserted a "test case" to the ticket that will reproduce the issue without test hooks but it would not really be a test I would want to check in. The problem is that there are multiple scenarios that can cause this to occur. Three different threads that would need to have test hooks to sync each other at specific points. The ack reader thread can be stuck on a socket read() or it can have already processed the header for the read, or it could be interrupted right before the read or it could be dead. All the while the dispatcher thread could be running and spawn a new ack reader thread (if it were dead) on a new connection or sending off a new batch. The gateway stopper thread could possibly send a close connectio on the old socket or if the ack reader thread was recreated, it would send on the new socket. All those scenarios now are invalid... we have changed the shut down to shutdown the dispatcher thread (not allowing it to spawn a new ack reader thread if it's shutting down). The connection is also being closed which will force the ack reader to break out of the read() or prevent it from reading anything else from the input stream. All the new test hooks would be added and some of these scenarios are now unable to be hit. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49102/#review140051 ----------------------------------------------------------- On June 22, 2016, 5:52 p.m., Jason Huynh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49102/ > ----------------------------------------------------------- > > (Updated June 22, 2016, 5:52 p.m.) > > > Review request for geode, anilkumar gingade, Barry Oglesby, nabarun nag, Dan > Smith, and xiaojian zhou. > > > Repository: geode > > > Description > ------- > > When closing a sender, the close connection message is sent on the same > connection that is used by the ack reader thread. This causes an issue as > two threads are now reading off the same socket concurrently. The fix is to > prevent this from happening but to do so, the input stream needs to be closed > (to free up from a socket read()). > The dispatcher also needs to shut down before the close connection is sent > out or it will spawn off another ack reader thread. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/AbstractGatewaySenderEventProcessor.java > ce08e8d > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/parallel/ConcurrentParallelGatewaySenderEventProcessor.java > 07a3be5 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/wan/serial/ConcurrentSerialGatewaySenderEventProcessor.java > ff810ec > > geode-wan/src/main/java/com/gemstone/gemfire/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java > b178192 > > Diff: https://reviews.apache.org/r/49102/diff/ > > > Testing > ------- > > > Thanks, > > Jason Huynh > >
