1ft Startup

Recently, I’ve made a few embarrassingly obvious concurrent-programming faux pas, so as a means of retribution to the multi-threaded daemon, I’ll try to confess a couple of my own anti-patterns here.

1. Servlets are reusable.

Here’s a snip of sample code:

public class MyServlet extends HttpServlet
{
  private HttpServletRequest _request;
  private HttpServletResponse _response;

  public void doGet(HttpServletRequest req, HttpServletResponse res)
  {
    _request = req;
    _response = res;
    ...
    new ResponseHandler().handle(this);
    ...
  }

  public HttpServletRequest getRequest() { return _request; }
  public HttpServletResponse getResponse() { return _response; }
  ...
}

What’s the most awful thing about this code? The big catch is that the servlet engine (Tomcat in my case) will reuse the same MyServlet object for subsequent requests, and when you make it big (or when someone’s, ahem, excessively aggressive crawler finds you), the servlet engine will call the same doGet()/doPost() on request B before request A one finishes. This means that, in the above example, when the ResponseHandler object calls MyServlet.getRequest() or MyServlet.getResponse(), it could potentially return the wrong object. This means that requests A and B could end up with their schwartz twisted, and you’ll probably end up with something like both responses appended together in B’s output stream, with A’s response lost and gone forever.

The solution is pretty simple in this case: Don’t use member variables in your servlet. Here’s the same code fixed:

public class MyServlet extends HttpServlet
{

  public void doGet(HttpServletRequest req, HttpServletResponse res)
  {
    ...
    new ResponseHandler().handle(req, res);
    ...
  }

  ...
} 

You’ll probably find you don’t even need the servlet object anymore and can pass its parts in. That thing is dangerous anyway.

2. Protect your cron jobs

This you can file under the “like, duh” folder, just don’t file it under the “my script only takes a few seconds to run, it can’t happen to me” category, like someone writing this entry did. Long-running jobs can get backed up on each-other, as cron will obediently lob another program out of the gate, whether or not the first one finishes. If you’ve got a serious hang-up or delay, this backup can get pretty severe. Take a flip through David Pashley’s excellent article on writing robust shell scripts and you’ll find something along these lines for that you can pretty much rip off and use:

if ( set -o noclobber; echo "$$" > "$lockfile") 2> /dev/null;
then
  trap 'rm -f "$lockfile"; exit $?' INT TERM EXIT

  [critical-section]

  rm -f "$lockfile"
  trap - INT TERM EXIT
else
  echo "Failed to acquire lockfile: $lockfile."
  echo "Held by $(cat $lockfile)"
fi

When you think you’ve fully redeemed yourself, and you can’t ever be totally sure, you absolutely must test your code. Testing for race conditions is notoriously difficult — for a start, I’d recommend some free load testing software like Apache JMeter, unless you have the megabucks for something like LoadRunner. JMeter seems like it can handle both web and non-web apps, including trying to injure JUnit tests with multithreaded calls. Very handy.

One quick concluding word about race conditions: You never think they’ll happen to you, and they don’t. Until they do, and when they hit, they’ll take down your entire site with hundreds of megabytes of logged stacktraces, at 3:30 in the morning, on your first day of vacation. This is not an exaggeration, it’s just life.