tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <wbar...@wilshire.com>
Subject Re: cvs commit: jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net PoolTcpEndpoint.java
Date Fri, 12 Sep 2003 18:32:55 GMT

----- Original Message -----
From: "Remy Maucherat" <remm@apache.org>
To: "Tomcat Developers List" <tomcat-dev@jakarta.apache.org>
Sent: Friday, September 12, 2003 1:38 AM
Subject: Re: cvs commit:
jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net
PoolTcpEndpoint.java


> Bill Barker wrote:
> >>On 12 Sep 2003, <billbarker@apache.org> wrote:
> >>
> >>
> >>> +++ PoolTcpEndpoint.java 12 Sep 2003 03:51:36 -0000 1.16
> >>> @@ -389,12 +389,12 @@
> >>>              if (accepted != null) {
> >>>                  try {
> >>>                      accepted.close();
> >>> -                    accepted = null;
> >>>                  } catch(Exception ex) {
> >>>                      msg = sm.getString("endpoint.err.nonfatal",
> >>>                                         accepted, ex);
> >>>                      log.warn(msg, ex);
> >>>                  }
> >>> +                accepted = null;
> >>>              }
> >>>
> >>>              if( ! running ) return null;
> >>
> >>wouldn't it be better to put the "accepted = null" into a finally
> >>block so you clean up even in the (unlikely but possible) case where
> >>close throws an Error (or Throwable) instead of an Exception?
> >
> > Wouldn't do anything.  The 'accepted' variable is local to the
stack-frame,
> > so it goes away if I throw clear of the method.  In this case you just
get a
> > DoS condition where no threads are listening on the ServerSocket.  I
briefly
> > thought about changing the catch to 'Throwable', but is it really
possible
> > for Socket.close to throw anything other than an Exception?
>
> I don't know. From traces I saw, there are conditions where there could
> be no thread listening on the socket (at that point, the connector was
> dead, of course), while everything else in the TP was looking ok
> (including no deadlock). There didn't seem to be anything in the logs
> related to an error during accept.

If you max out the threads, then the thread that is waiting in ThreadPool is
the one that will do the next accept.  It's supposed to be a temporary
condition (since a thread should get returned to to Pool soon).  Porting the
connectionTimeout throttling back to 4.1 should make it more responsive in
this case (since Tomcat will stop doing keep-alives).  Of course, what it's
really telling you is that your maxProcessors is set too low :).

>
> I'd catch throwable just to be safe, personally :)

I'll catch throwable then.

>
> Remy
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>


Mime
View raw message