Hi, 2017-03-29 23:59 GMT+03:00 Mark Thomas <ma...@apache.org>: > > On 29/03/17 08:02, Violeta Georgieva wrote: > >> 2017-02-27 16:50 GMT+02:00 Mark Thomas <ma...@apache.org>: > > <snip/> > > >>> Overall, I like the approach and would support apply a patch along these > >>> lines once the timing issues are resolved. > > > > Any feedback about the new changes? > > First of all, kudos for implementing what is potentially complex > processing cleanly. It makes reviewing it much easier. > > I think the fix for the previous timing issue has introduced a different > one. Consider the following scenario. > > - thread A is in onDataAvailable() processing data > - thread B calls suspendonDataAvailable() > - thread A exits onDataAvailable() > - thread A enters the if statement and prepares to return SUSPENDED > - thread B calls resume which is a NO-OP because the reading flag is set > - thread A enters finally block and clears reading flag > - thread A returns SUSPENDED > > and processing hangs. > > I think the problem is that "reading in progress" and suspended are > treated separately when it is the combination of the two that represents > the current state. > > If it were me, I'd probably look at adding a state machine (but that is > just how I tend to approach these sorts of problems).
You are right. I reworked the patch, the new implementation is using a state machine. The changes are available here https://github.com/apache/tomcat/pull/42 Can you please take a look? Thanks for the review, Violeta > Mark > > > > > > Thanks for the review, > > Violeta > >> > >>> <snip/> > >>> > >>>>>> This approach would mean some internal API changes but that is fine > > for > >>>> 9.0.x and I don't see a problem with 8.5.x either. Whether this is > >>>> back-ported to 8.0.x and 7.0.x is TBD. It also opens up the > > possibility of > >>>> being able to suspend/resume other protocols but I haven't thought a > > great > >>>> deal about how that might work. > >>>>> > >>>>> I need this functionality only for 9.0.x and 8.5.x. > >>> > >>> Then let's not back-port this any earlier than 8.5.x. > >>> > >>> Mark > >>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > >>> For additional commands, e-mail: dev-h...@tomcat.apache.org > >>> > >> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org >
