Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-3175 Fix possible file name conflict during consent change #2113

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

- [FIX] Fix sporadic file overwrite during consent change, ensuring event data integrity. See [#2113][]

# 2.20.0 / 14-11-2024

- [FIX] Fix race condition during consent change, preventing loss of events recorded on the current thread. See [#2063][]
Expand Down Expand Up @@ -793,6 +795,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
[#2104]: https://github.com/DataDog/dd-sdk-ios/pull/2104
[#2099]: https://github.com/DataDog/dd-sdk-ios/pull/2099
[#2063]: https://github.com/DataDog/dd-sdk-ios/pull/2063
[#2113]: https://github.com/DataDog/dd-sdk-ios/pull/2113
[#2092]: https://github.com/DataDog/dd-sdk-ios/pull/2092
[@00fa9a]: https://github.com/00FA9A
[@britton-earnin]: https://github.com/Britton-Earnin
Expand Down
32 changes: 29 additions & 3 deletions DatadogCore/Sources/Core/Storage/FilesOrchestrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ internal protocol FilesOrchestratorType: AnyObject {

/// Orchestrates files in a single directory.
internal class FilesOrchestrator: FilesOrchestratorType {
enum Constants {
/// Precision in which the timestamp is stored as part of the file name.
static let fileNamePrecision: TimeInterval = 0.001 // millisecond precision
}

/// Directory where files are stored.
let directory: Directory
/// Date provider.
Expand Down Expand Up @@ -109,7 +114,7 @@ internal class FilesOrchestrator: FilesOrchestratorType {
// happens too often).
try purgeFilesDirectoryIfNeeded()

let newFileName = fileNameFrom(fileCreationDate: dateProvider.now)
let newFileName = nextFileName()
let newFile = try directory.createFile(named: newFileName)
lastWritableFileName = newFile.name
lastWritableFileObjectsCount = 1
Expand All @@ -118,6 +123,27 @@ internal class FilesOrchestrator: FilesOrchestratorType {
return newFile
}

/// Generates a unique file name based on the current time, ensuring that the generated file name does not already exist in the directory.
/// When a conflict is detected, it adjusts the timestamp by advancing the current time by the precision interval to ensure uniqueness.
///
/// In practice, name conflicts are extremely unlikely due to the monotonic nature of `dateProvider.now`.
/// Conflicts can only occur in very specific scenarios, such as during a tracking consent change when files are moved
/// from an unauthorized (.pending) folder to an authorized (.granted) folder, with events being written immediately before
/// and after the consent change. These conflicts were observed in tests, causing flakiness. In real-device scenarios,
/// conflicts may occur if tracking consent is changed and two events are written within the precision window defined
/// by `Constants.fileNamePrecision` (1 millisecond).
private func nextFileName() -> String {
var newFileName = fileNameFrom(fileCreationDate: dateProvider.now)
while directory.hasFile(named: newFileName) {
// Advance the timestamp by the precision interval to avoid generating the same file name.
// This may result in generating file names "in the future", but we aren't concerned
// about this given how rare this scenario is.
let newDate = dateProvider.now.addingTimeInterval(Constants.fileNamePrecision)
newFileName = fileNameFrom(fileCreationDate: newDate)
}
return newFileName
}

private func reuseLastWritableFileIfPossible(writeSize: UInt64) -> WritableFile? {
if let lastFileName = lastWritableFileName {
if !directory.hasFile(named: lastFileName) {
Expand Down Expand Up @@ -289,14 +315,14 @@ internal class FilesOrchestrator: FilesOrchestratorType {
/// File creation date is used as file name - timestamp in milliseconds is used for date representation.
/// This function converts file creation date into file name.
internal func fileNameFrom(fileCreationDate: Date) -> String {
let milliseconds = fileCreationDate.timeIntervalSinceReferenceDate * 1_000
let milliseconds = fileCreationDate.timeIntervalSinceReferenceDate / FilesOrchestrator.Constants.fileNamePrecision
let converted = (try? UInt64(withReportingOverflow: milliseconds)) ?? 0
return String(converted)
}

/// File creation date is used as file name - timestamp in milliseconds is used for date representation.
/// This function converts file name into file creation date.
internal func fileCreationDateFrom(fileName: String) -> Date {
let millisecondsSinceReferenceDate = TimeInterval(UInt64(fileName) ?? 0) / 1_000
let millisecondsSinceReferenceDate = TimeInterval(UInt64(fileName) ?? 0) * FilesOrchestrator.Constants.fileNamePrecision
return Date(timeIntervalSinceReferenceDate: TimeInterval(millisecondsSinceReferenceDate))
}
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,31 @@ class FilesOrchestratorTests: XCTestCase {
XCTAssertNil(try? orchestrator.directory.file(named: file2.name))
}

func testWhenFileAlreadyExists_itWaitsAndCreatesFileWithNextName() throws {
let date: Date = .mockDecember15th2019At10AMUTC()
let dateProvider = RelativeDateProvider(
startingFrom: date,
advancingBySeconds: FilesOrchestrator.Constants.fileNamePrecision
)

// Given: A file with the current time already exists
let orchestrator = configureOrchestrator(using: dateProvider)
let existingFile = try orchestrator.directory.createFile(named: fileNameFrom(fileCreationDate: date))

// When: The orchestrator attempts to create a new file with the next available name
let nextFile = try orchestrator.getWritableFile(writeSize: 1)

// Then
let existingFileDate = fileCreationDateFrom(fileName: existingFile.name)
let nextFileDate = fileCreationDateFrom(fileName: nextFile.name)
XCTAssertNotEqual(existingFile.name, nextFile.name, "The new file should have a different name than the existing file")
XCTAssertGreaterThanOrEqual(
nextFileDate.timeIntervalSince(existingFileDate),
FilesOrchestrator.Constants.fileNamePrecision,
"The timestamp of the new file should be at least `fileNamePrecision` later than the existing file"
)
}

// MARK: - Readable file tests

func testGivenNoReadableFiles_whenObtainingFiles_itReturnsEmpty() {
Expand Down