Common Bug In Retry Logic

The “for”, “while” and “do-while” statements in programming languages repeat application logic. Those statement are often used for implementing the retry logic to deal with some types of errors. Programmers often embed two pieces of logic in the same loop statement: one repeats N times and the second N-1 times. For example, concatenation of a string array to produce the comma-separated output joins N array elements with N-1 commas in between. While doing code reviews I have seen one bug many times during my career so I decided to write about it. In this post, I will show broken and correct code examples of simple retry logic.

The code below opens a file using simple retry logic. If the file does not exist the function waits for 100 milliseconds and tries opening the file again. This method has the tryCount parameter to control the number of attempts to open the file. Don’t copy/paste the code as it contains a bug:

   public BufferedReader tryOpenFile1(String fileName, int tryCount) throws IOException {
      Path path = FileSystems.getDefault().getPath(fileName);
      while (tryCount-- > 0) {
         System.out.println("Check if file exists. Try " + tryCount);
         if (Files.exists(path)) {
            return Files.newBufferedReader(path, Charset.forName("UTF-8"));
         }
         sleep(100);
      }
      return null;
   }

   protected void sleep(int milliseconds) {
      try {
         System.out.println(String.format("Sleeping for %s milliseconds", milliseconds));
         Thread.sleep(milliseconds);
      } catch (InterruptedException e) {
         e.printStackTrace();
      }
   }

The tryOpenFile1 method works and commonly written unit-tests pass, but this method contains a bug. The method repeats “sleeping” logic N times when it should do that N-1 times. This bug leads to the extra sleeping time period after the last unsuccessful attempt occurred to open the file. This can be fixed by wrapping the “sleep(100);” statement with a separate IF statement to repeat the logic only N-1 times:

   public BufferedReader tryOpenFile2(String fileName, int tryCount) throws IOException {
      Path path = FileSystems.getDefault().getPath(fileName);
      while (tryCount-- > 0) {
         System.out.println("Check if file exists. Try " + tryCount);
         if (Files.exists(path)) {
            return Files.newBufferedReader(path, Charset.forName("UTF-8"));
         }
         if (tryCount > 0) {
            sleep(100);
         }
      }
      return null;
   }

The re-try logic is very common in old and modern applications. Even though the implementation looks simple, the bug rate of the retry logic is surprisingly high in practice.

I hope reading this post was useful. Try to avoid the described bug in your loop implementations. Remember about N and N-1 logic in the loops.