On 11/03/2015 18:13, Christopher Schultz wrote: > Rainer, > > On 3/11/15 12:38 PM, Rainer Jung wrote: >> Am 11.03.2015 um 15:45 schrieb Mark Thomas: >>> On 11/03/2015 14:44, ma...@apache.org wrote: >>>> Author: markt >>>> Date: Wed Mar 11 14:44:23 2015 >>>> New Revision: 1665888 >>>> >>>> URL: http://svn.apache.org/r1665888 >>>> Log: >>>> Fix 57653. Crash when multiple events for same socket are returned >>>> via separate apr_pollfd_t structures >>> >>> Review appreciated from folks that understand C better than I do (i.e. >>> pretty much anybody). Hopefully it is clear from the comment what I am >>> trying to do here and why. >> >> I guess it is more of a question of knowing the pollset and RING magic - >> which I don't really qualify for - but it looks good to me. > > +1 > >> Even if for >> the two fd with the same socket their client_data parts might not be >> identical, they should point to the same (identical) socket structure, >> so NULLing "pe" in the socket during the first fd occurence should work >> as a decision point for the second occurence. > > +1 > > Is it a problem to call apr_pollset_remove for that file descriptor if > it's not actually in the pollset? That is, are remove operations > idempotent? Would it be considered an error (at a higher level) to try > to remove the same fd twice?
You get a return code indicating that the fd wasn't in the pollset. >> The only other case would be some fd which has a NUL s->pe from the >> beginning, but then the existing code would have also crashed though >> only after calling apr_pollset_remove(). So I guess that case is not >> possible. > > +1 > > What you (Mark) have done is just avoid a seg fault when s->pe is NULL. Which should never happen apart from in the case that we are trying to handle. > What you haven't done (which might not actually be necessary) is solve > any issue where this method shouldn't be called in the first place (i.e. > state management). I don't believe that is an issue. We could have tried to merge multiple returns for the one fd but that would have been a much more complicated patch. I'd rather deal with that in the Java code. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
