From c7cbebb5345d9c7692ba56925e93069640fe9a40 Mon Sep 17 00:00:00 2001 From: Christopher Batey Date: Thu, 4 Jan 2018 12:37:16 +0000 Subject: [PATCH] Fix record overflow race condition in flight recorder #21992 The flag used to show a record is dirty/commited was overwritten by the last 4 bytes of the previous metadata. Most of the time this isn't caught as the record is written fixed width and typically the last bytes are 0s which is the same as the Commited flag. However under concurrency this can overwrite the Dirty flag that is preventing corruption. --- .../src/main/scala/akka/remote/artery/FlightRecorder.scala | 5 +++-- .../test/scala/akka/remote/artery/FlightRecorderSpec.scala | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/akka-remote/src/main/scala/akka/remote/artery/FlightRecorder.scala b/akka-remote/src/main/scala/akka/remote/artery/FlightRecorder.scala index 50839d067f..737d636445 100644 --- a/akka-remote/src/main/scala/akka/remote/artery/FlightRecorder.scala +++ b/akka-remote/src/main/scala/akka/remote/artery/FlightRecorder.scala @@ -152,7 +152,7 @@ private[remote] class RollingEventLogSection( * sane way to use the same code here and in the test, too. */ def write(logId: Int, recordBuffer: ByteBuffer): Unit = { - val logBuffer = buffers(logId) + val logBuffer: MappedResizeableBuffer = buffers(logId) @tailrec def writeRecord(): Unit = { // Advance the head @@ -162,7 +162,8 @@ private[remote] class RollingEventLogSection( // if the head *wraps over* and points again to this location. Without this we would end up with partial or corrupted // writes to the slot. if (logBuffer.compareAndSetInt(recordOffset, Committed, Dirty)) { - logBuffer.putBytes(payloadOffset, recordBuffer, recordSize) + // 128 bytes total, 4 bytes used for Commit/Dirty flag + logBuffer.putBytes(payloadOffset, recordBuffer, recordSize - 4) //println(logBuffer.getLong(recordOffset + 4)) // Now this is free to be overwritten diff --git a/akka-remote/src/test/scala/akka/remote/artery/FlightRecorderSpec.scala b/akka-remote/src/test/scala/akka/remote/artery/FlightRecorderSpec.scala index 78ba8ec423..9601bd0718 100644 --- a/akka-remote/src/test/scala/akka/remote/artery/FlightRecorderSpec.scala +++ b/akka-remote/src/test/scala/akka/remote/artery/FlightRecorderSpec.scala @@ -358,10 +358,10 @@ class FlightRecorderSpec extends AkkaSpec { channel.force(false) reader.rereadStructure() - reader.structure.loFreqLog.logs(0).richEntries.size should ===(FlightRecorder.LoFreqWindow) + reader.structure.loFreqLog.logs.head.richEntries.size should ===(FlightRecorder.LoFreqWindow) for (i ← 1 to Threads) { - val entries = reader.structure.loFreqLog.logs(0).richEntries.filter(_.code == i).toSeq + val entries = reader.structure.loFreqLog.logs.head.richEntries.filter(_.code == i).toSeq entries.exists(_.dirty) should be(false) // Entries are consecutive for any given writer