[GH-ISSUE #377] Log file rotation looks dangerous #272

New issue

Open

opened 2026-02-28 01:23:57 +03:00 by kerem · 0 comments

kerem commented

2026-02-28 01:23:57 +03:00

Owner

Originally created by @alxndrsn on GitHub (Apr 6, 2016).
Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/377

Observed

FileManager.rotate() looks like it will have unpredictable results:

The “rotation” occurs, on a separate thread, as follows:

the entire logfile is read to count the number of lines
the log file is opened again, and the first 30% of the lines are read and discarded
the remaining 70% of the old log file is read, and written to the new file

The PrintWriter for writing logFile is created on the original thread, so it may or may not be created before the rotation is completed. It is opened in append mode, and it’s not immediately clear which file this will write to (new or old), and if the Writer will point to a deleted file if the execution order is as follows:

PrintWriter created, pointing to smssync_log
smssync_log.new created, and moved to smssync_log

Because the rotation is happening on a separate thread, there may be concurrent writes to smssync_log while it is also being read and rewritten to the smssync_log.new file.

Suggested fixes

One of the following might improve things:

create a new file each run. You may want to discard e.g. all but the last 10 log files
discard previous log file on startup
use a third party library with support for log rotation or log file size limits

Notes

Due to https://github.com/ushahidi/SMSSync/issues/375, the rotate() method will currently never be called.

Originally created by @alxndrsn on GitHub (Apr 6, 2016). Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/377 # Observed [`FileManager.rotate()`](https://github.com/ushahidi/SMSSync/blob/master/smssync/src/main/java/org/addhen/smssync/data/cache/FileManager.java#L72) looks like it will have unpredictable results: The “rotation” occurs, on a separate thread, as follows: 1. the entire logfile is read to count the number of lines 2. the log file is opened again, and the first 30% of the lines are read and discarded 3. the remaining 70% of the old log file is read, and written to the new file The `PrintWriter` for writing `logFile` is created on the original thread, so it may or may not be created before the rotation is completed. It is opened in append mode, and it’s not immediately clear which file this will write to (new or old), and if the `Writer` will point to a deleted file if the execution order is as follows: 1. `PrintWriter` created, pointing to `smssync_log` 2. `smssync_log.new` created, and moved to `smssync_log` Because the rotation is happening on a separate thread, there may be concurrent writes to `smssync_log` while it is also being read and rewritten to the `smssync_log.new` file. # Suggested fixes One of the following might improve things: 1. create a new file each run. You may want to discard e.g. all but the last 10 log files 2. discard previous log file on startup 3. use a third party library with support for log rotation or log file size limits # Notes Due to https://github.com/ushahidi/SMSSync/issues/375, the `rotate()` method will currently never be called.