+1 unit tests for all transitions ;-) On Tue, Dec 6, 2011 at 3:05 PM, Julien Vermillard <[email protected]>wrote:
> On Tue, Dec 6, 2011 at 7:26 PM, Emmanuel Lecharny <[email protected]> > wrote: > > Hi, > > > > following the different states a session can be in, plus the possible > > transitions from one state to another, we will have an issue if we don't > > protect the sessions state against concurrent access and modifications. > > > > For instance, as right now, the session's state is a volatile variable in > > AbstractIoSession : > > > > protected volatile SessionState state; > > > > it looks like it's protected. It is, only of we consider it from a > > read/write pespective. That mean it's safe to read the sate, it's safe to > > change it, there is no way we can't do that as an atomic operation. > > > > But there is something we can't do, it's changing the state *and* check > that > > the transition is ok : > > > > if (state == CONNECTED ) { > > state = SECURING > > } > > > > for instance, might fail as the session's state may have changed before > we > > try to change it. > > > > So we need to protect the state transition from concurrent accesses. > Again, > > one possible solution would be to use a ReentrantReadWrite lock, to allow > > fast session's state reads, and safe transition. > > > > I would also suggest that we have only one way to change the state, > throw a > > method like : > > > > void changeState( SessionState from, SessionState to) > > > > in order to be able to check that the transition does not violate the > table > > of possible transitions. > > > > wdyt ? > > > + 1 > anyway I'm not sure it's handled correctly today (who said unit tests ?) >
