Hi all,






While investigating an intermittent message-loss report in Spring
Integration's ApacheCommonsFileTailingMessageProducer (which wraps Tailer),
we traced it to a bug in Tailer.run()'s rotation-detection branch. Filing
this here first since it looks related to the still-open
https://issues.apache.org/jira/browse/IO-399, and wanted to share the
analysis in case it's useful, or points to a shared fix for both. Plus, it
looks like we cannot create JIRA accounts to open a new ticket or comment
on the opened one.






  The code (Tailer.java, current run(), the length < position "file was
rotated" branch):






  if (length < position) {



      // File was rotated



      listener.fileRotated();



      // Reopen the reader after rotation ensuring that the old file is
closed iff we re-open it


      // successfully



      try (RandomAccessResourceBridge save = reader) {



          reader = tailable.getRandomAccess(RAF_READ_ONLY_MODE);



          // At this point, we're sure that the old file is rotated



          // Finish scanning the old file and then we'll start with the new
one


          try {



              readLines(save);



          } catch (final IOException ioe) {



              listener.handle(ioe);



          }



          position = 0;



      } catch (final FileNotFoundException e) {



          // in this case we continue to use the previous reader and
position values


          listener.fileNotFound();



          ThreadUtils.sleep(delayDuration);



      }



      continue;



  }







  The bug: if the reopen tailable.getRandomAccess(RAF_READ_ONLY_MODE)
throws FileNotFoundException — which happens naturally during ordinary
rotation, when the old file has just been moved/removed but the replacement
hasn't been created yet — the comment says the code "continues to use the
previous reader and position values." It doesn't: the enclosing try
RandomAccessResourceBridge save = reader) { ... } already closed that
reader (as save) before the reopen attempt failed, because the assignment
reader = ... never completed. reader still refers to that  now-closed
object.







  On the next iteration, once the replacement file grows past the stale
position, the loop takes the length > position branch and calls
readLines(reader) on the closed resource. That throws, and it's caught by
the outer catch (final Exception e) { listener.handle(e); } in run()'s
 top-level try — which does not rethrow. run() proceeds to finally { ...;
close(); } and returns. The tailing thread is gone for good, with no
indication beyond whatever the TailerListener.handle(Exception) callback
happened to do with it (in our case, publishing an event nobody  was
necessarily watching).







  Net effect: a routine, transient "file briefly absent during rotation"
window can permanently and silently stop a Tailer, with no automatic
recovery, no exception surfaced to the caller, and no correction of the
stale reader/position state as the comment claims.




  Relation to IO-399: that issue describes the same class of failure
(uncaught/unhandled FileNotFoundException during a rotation-triggered
reopen killing the thread) in the sibling reOpen=true periodic-reopen path,
which today still has no try/catch around its getRandomAccess()  call at
all:







  if (getRun() && reOpen) {



      reader = tailable.getRandomAccess(RAF_READ_ONLY_MODE);



      reader.seek(position);



  }







  Both point at the same underlying gap: Tailer doesn't have a robust way
to say "the file is momentarily gone, keep the old state and retry" without
either (a) closing a resource it then tries to keep using, or (b) not
catching the failure at all.




  Suggested direction: in the length < position branch, don't let the
try-with-resources close the old reader until a replacement has actually
been successfully opened — e.g. only swap save/reader after
getRandomAccess() succeeds, or restructure so the FileNotFoundException
catch reopens/reassigns a genuinely usable reader (or explicitly nulls it
out and lets the "reader == null" top-of-loop reopen logic take over)
rather than leaving a closed handle referenced by a live variable.





  Workaround on our side:
https://github.com/spring-projects/spring-integration/pull/11381






  Thanks for maintaining commons-io — let us know if a minimal standalone
reproducer would help, we can put one together!

Regards,
Artem Bilan
Spring Framework Team
Broadcom

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to