[concurrency-interest] Simple ScheduledFuture problem

Tim Peierls tim at peierls.net
Tue Aug 22 09:42:07 EDT 2006


You are creating a new thread pool each time the PingTask runs. I was
suggesting that the thread pool be a field, maybe even a static field, of
PingTask.

--tim

On 8/22/06, robert lazarski <robertlazarski at gmail.com> wrote:
>
> I finally got some time to implement the latest suggestions of David
> and Tim. This is what I came up with:
>
> class PingTask implements Runnable {
>
>          public void run() {
>
>              Future f = null;
>              boolean interrupted = false;
>              try {
>                  ExecutorService exec = Executors.newCachedThreadPool();
>                  f = exec.submit(new PingFuture());
>                  f.get(5000, TimeUnit.MILLISECONDS);
>
>              } catch (TimeoutException ex) {
>                    interrupted = true;
>                    logger.error("Future timed out: \n" + ex.getMessage(),
> ex);
>              } catch (InterruptedException ex) {
>                    interrupted = true;
>                    logger.error ("Future interrupted: \n" + ex.getMessage(),
> ex);
>              } catch (Exception ex) {
>                    logger.error(ex.getMessage(), ex);
>              }
>              finally {
>                  f.cancel(true);
>                  logger.debug("CURRENT STATE: " + getMessage(flag));
>
>              }
>              if (interrupted) {
>                  Thread.currentThread().interrupt();
>              }
>          }
>      } // end inner class PingTask
>
> Thanks for the feedback!
> Robert
>
> On 8/17/06, David Holmes <dcholmes at optusnet.com.au> wrote:
> > Robert,
> >
> > A concern with this is what happens to the thread when the connection
> does
> > not respond? You have the get() timeout but the thread you used (whether
> > created directly or via an Executor) may still get hung on the actual
> > operation. You might want to cancel/interrupt the thread after you
> timeout.
> >
> > Cheers,
> > David Holmes
> >
> > > -----Original Message-----
> > > From: concurrency-interest-bounces at cs.oswego.edu
> > > [mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of robert
> > > lazarski
> > > Sent: Friday, 18 August 2006 12:56 AM
> > > To: Joe Bowbeer
> > > Cc: concurrency-interest at cs.oswego.edu
> > > Subject: Re: [concurrency-interest] Simple ScheduledFuture problem
> > >
> > >
> > > Thanks Dave and Joe. I implemented the changes you two mentioned (I
> > > hope) and now invoke a FutureTask inside the Runnable of the
> > > ScheduledFuture . Here's what I came up with. The purpose here is just
>
> > > to simply record the states of an app server.
> > >
> > > Thanks!
> > > Robert
> > >
> > > package org;
> > >
> > > import java.io.BufferedReader;
> > > import java.io.IOException;
> > > import java.io.InputStream;
> > > import java.io.InputStreamReader;
> > > import java.net.InetAddress;
> > > import java.net.Socket;
> > > import java.net.URL;
> > > import java.net.URLConnection;
> > > import java.util.HashMap;
> > > import java.util.Map;
> > > import java.util.concurrent.Executors;
> > > import java.util.concurrent.FutureTask ;
> > > import java.util.concurrent.ScheduledExecutorService;
> > > import java.util.concurrent.ScheduledFuture;
> > > import java.util.concurrent.TimeUnit;
> > >
> > > import org.apache.commons.logging.Log ;
> > > import org.apache.commons.logging.LogFactory;
> > >
> > > public class Ping {
> > >
> > >       private volatile int  flag = 0;
> > >       private static int STOPPED = new Integer(0);
> > >       private static int BEGIN_INIT = new Integer(1);
> > >       private static int RUNNING = new Integer(2);
> > >           private static Map <Integer, String> map;
> > >
> > >       /** commons logging declaration. */
> > >       private static Log logger = LogFactory.getLog(
> > >                 Ping.class);
> > >
> > >       public static void main(String[] args) throws Exception {
> > >
> > >               new Ping();
> > >       }
> > >
> > >       Ping () {
> > >           ScheduledExecutorService ses =
> > > Executors.newSingleThreadScheduledExecutor();
> > >           // Do pings, starting now, with a 2 second delay in between
> > >           ScheduledFuture <?> ping =
> > > ses.scheduleWithFixedDelay(new PingTask(),
> > >               0L, 2000L, TimeUnit.MILLISECONDS);
> > >       }
> > >
> > >       static {
> > >           map = new HashMap<Integer, String>();
> > >           map.put(STOPPED, "STOPPED");
> > >           map.put(BEGIN_INIT, "BEGIN_INIT");
> > >           map.put(RUNNING, "RUNNING");
> > >       }
> > >
> > >       private synchronized void setFlag(int var) {
> > >           flag = var;
> > >       }
> > >
> > >       /** Get message via an int.
> > >        * @param code Integer mapped to message
> > >        * @return String mapped message
> > >        */
> > >       public final String getMessage(int code) {
> > >           return map.get(code);
> > >       }
> > >
> > >       class PingFuture implements Runnable {
> > >           private void doConnect(String host, int port, boolean
> > > on_connect, int state) {
> > >               try {
> > >                   logger.debug("connecting to host: " +host+ ",
> > > port: " + port);
> > >                   InetAddress addr = InetAddress.getByName (host);
> > >                   // will throw an exception if could not connect
> > >                   Socket s = new Socket(addr, port);
> > >                   s.close();
> > >                   if (on_connect) {
> > >                       logger.debug("Found port: " + port);
> > >                       setFlag(state);
> > >                   }
> > >
> > >               } catch (Exception ex) {
> > >                   logger.error("Can't find port: " + port);
> > >                   logger.error(ex.getMessage(), ex);
> > >                   if (!on_connect) {
> > >                       setFlag(state);
> > >                   }
> > >               }
> > >
> > >           }
> > >
> > >           private void doConnect(URL host, boolean on_connect, int
> > > state) throws IOException {
> > >               InputStream is = null;
> > >               BufferedReader in = null;
> > >               try {
> > >                   logger.debug("connecting to url: " + host.toString());
> > >                   URLConnection uc = host.openConnection();
> > >                   if (uc == null) {
> > >                       logger.error("Got a null URLConnection
> object!");
> > >                       return;
> > >                   }
> > >                   is = uc.getInputStream();
> > >                   if (is == null) {
> > >                       logger.error ("Got a null content object!");
> > >                       return;
> > >                   }
> > >                   in = new BufferedReader(new InputStreamReader(
> > >                         is));
> > >                   String inputLine;
> > >                   // just test that its readable for now
> > >                   while ((inputLine = in.readLine()) != null)  {
> > >                         ;
> > >                   }
> > >                   if (on_connect) {
> > >                       logger.debug("Found url: " + host.toString());
> > >                       setFlag(state);
> > >                   }
> > >
> > >               } catch (Exception ex) {
> > >                   logger.error("Can't find url: " + host.toString());
> > >                   logger.error(ex.getMessage(), ex);
> > >                   if (!on_connect) {
> > >                       setFlag(state);
> > >                   }
> > >               } finally {
> > >                   if (is != null) {
> > >                       is.close();
> > >                   }
> > >                   if (in != null) {
> > >                       in.close();
> > >                   }
> > >               }
> > >
> > >           }
> > >
> > >           public void run() {
> > >
> > >               try {
> > >                   if (flag == STOPPED) {
> > >                       doConnect("localhost", 1099, true, BEGIN_INIT);
> > >                   }
> > >                   if (flag == BEGIN_INIT) {
> > >                       // test state of stopped to prevent endless loop
> > >                       doConnect("localhost", 1099, false, STOPPED);
> > >                       doConnect(new
> > > URL("http://localhost:8080/maragato/"), true, RUNNING);
> > >
> > >                   }
> > >                   if (flag == RUNNING) {
> > >                       doConnect(new
> > > URL("http://localhost:8080/maragato/"), false, STOPPED);
> > >                   }
> > >
> > >               } catch (Exception ex) {
> > >                     logger.error(ex.getMessage(), ex);
> > >               }
> > >           }
> > >       } // end inner class PingFuture
> > >
> > >       class PingTask implements Runnable {
> > >
> > >           public void run() {
> > >
> > >               try {
> > >                   FutureTask<?> f = new FutureTask<Object>(new
> > > PingFuture(), null);
> > >                   Thread thread = new Thread(f);
> > >                   thread.start ();
> > >                   // 5 seconds to finish connect or will timeout
> > >                   f.get(5000, TimeUnit.MILLISECONDS);
> > >               } catch (Exception ex) {
> > >                     logger.error(ex.getMessage(), ex);
> > >               }
> > >               finally {
> > >                   logger.debug("CURRENT STATE: " + getMessage(flag));
> > >
> > >               }
> > >           }
> > >       } // end inner class PingTask
> > >
> > > }
> > >
> > >
> > > On 8/16/06, Joe Bowbeer <joe.bowbeer at gmail.com > wrote:
> > > > At a glance, I notice a few things that could be cleaned up, though
> > > > they may have no impact on the problem.
> > > >
> > > > 1. setFlag is synchronized but there is no synchronized getFlag
> method.
> > > >
> > > > Either added synchronized getFlag, or declare flag to be "volatile".
> > > >
> > > > 2. doConnect(url) code may fail without closing input stream.
> > > >
> > > > Add try-catch after opening input stream.
> > > >
> > > > 3. Also, I would consider switching to fixed delay to avoid the
> > > > possibility of multiple outstanding pings.
> > > > _______________________________________________
> > > > Concurrency-interest mailing list
> > > > Concurrency-interest at altair.cs.oswego.edu
> > > > http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
> > > >
> > > _______________________________________________
> > > Concurrency-interest mailing list
> > > Concurrency-interest at altair.cs.oswego.edu
> > > http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
> >
> >
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at altair.cs.oswego.edu
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: /pipermail/attachments/20060822/1e0da407/attachment-0001.html 


More information about the Concurrency-interest mailing list