On 6 December 2011 18:26, 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)
should be protected? > > in order to be able to check that the transition does not violate the table > of possible transitions. > > wdyt ? +1, but won't be totally safe unless the field is made private, and a protected getter added. If the field is not part of the public API, that would be OK; otherwise it would not, as it breaks binary compat. In any case, I suggest deprecating the field to document that it should not be accessed directly. > -- > Regards, > Cordialement, > Emmanuel Lécharny > www.iktek.com >
