Skip to content

Commit 77230ec

Browse files
marcdumais-workbhufmann
authored andcommitted
[AsyncFileHandler] insure good synchronization for 'fRecordBuffer'
This field is accessed by various threads, whenever logging is happening. All accesses are synchronized except the empty check in timer thread, done before calling flush(). Once in a while, the record buffer is flushed to the writer queue and a new, empty buffer is created and assigned to "fRecordBuffer". If that happens just before the timer task does its empty check, that thread might still see the old buffer, and then the call to flush() might result in adding an empty buffer to the queue. To avoid that, the empty check has been moved inside the synchronized flush() function. Also, for explicitness sake, mark field fFileHandler as final. It's already used as such, and is only set in the constructor, but this makes it explicit. Signed-off-by: Marc Dumais <[email protected]>
1 parent 85839ff commit 77230ec

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

src/main/java/org/eclipse/tracecompass/traceeventlogger/AsyncFileHandler.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
*/
8888
public class AsyncFileHandler extends StreamHandler {
8989
private static final LogRecord CLOSE_EVENT = new LogRecord(Level.FINEST, "CLOSE_EVENT"); //$NON-NLS-1$
90-
private FileHandler fFileHandler;
90+
private final FileHandler fFileHandler;
9191
private BlockingQueue<List<LogRecord>> fQueue;
9292
private Thread fWriterThread;
9393
private int fMaxSize = 1024;
@@ -105,9 +105,7 @@ public class AsyncFileHandler extends StreamHandler {
105105
TimerTask fTask = new TimerTask() {
106106
@Override
107107
public void run() {
108-
if (!fRecordBuffer.isEmpty()) {
109-
flush();
110-
}
108+
flush();
111109
}
112110
};
113111

@@ -290,8 +288,10 @@ public synchronized void close() throws SecurityException {
290288
@Override
291289
public synchronized void flush() {
292290
try {
293-
fQueue.put(fRecordBuffer);
294-
fRecordBuffer = new ArrayList<>(fMaxSize);
291+
if (!fRecordBuffer.isEmpty()) {
292+
fQueue.put(fRecordBuffer);
293+
fRecordBuffer = new ArrayList<>(fMaxSize);
294+
}
295295
} catch (InterruptedException e) {
296296
Thread.currentThread().interrupt();
297297
}

0 commit comments

Comments
 (0)