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

Corrupted file after write using buffered sink (Kotlin/Native) #1586

Open
bcmedeiros opened this issue Jan 30, 2025 · 13 comments
Open

Corrupted file after write using buffered sink (Kotlin/Native) #1586

bcmedeiros opened this issue Jan 30, 2025 · 13 comments

Comments

@bcmedeiros
Copy link
Contributor

I have the following method running on Kotlin/Native linux/amd64:

    fun set(id: String, value: Long) {
        val bookmarkPath = path / id
        FileSystem.SYSTEM.sink(bookmarkPath).use { s ->
            s.buffer().use { b ->
                b.writeUtf8("$value\n")
            }
        }
    }

Even though the content being sent to writeUtf8 is always the output of a Long.toString() followed by a line break, one of the files that is manipulated by this method ended up having its content being 7 zero bytes.

The output of xxd is as below:

[bruno@bruno-desktop ~]$ xxd data/bookmark/bookmark.txt
00000000: 0000 0000 0000 00                        .......

There is also the original file attached in case the output below needs to be verified: bookmark.txt

I think this is some sort of bug in okio for a few reasons:

  1. There is no other method that could have written to this file
  2. value is of type Long, so I can't see how the toString() of a long could have produced 7 zero bytes, nor how could the \n could have been ignored before writeUtf8 is called.
  3. There is always a single copy of the process that could have written to this file, and that's managed by systemd. The file system is a block device, so no network involved.

This process is running for months, file always had a sequence of numerical chars followed by a \n. 2 days ago, though, for some still unknown reason, I found this file with the bogus content above.

Could anyone with good Okio knowledge help me investigate what could be causing that?

@JakeWharton
Copy link
Collaborator

Nothing jumps to mind. Is the length of the number being written 6 digits? So that plus the newline would account at least for the count of 7 zero bytes?

By the way, your code can be improved a bit by changing it to:

FileSystem.SYSTEM.sink(bookmarkPath).buffer().use { s ->
  b.writeDecimalLong(value)
  b.writeByte('\n'.code)
}

Which avoids creation of an intermediate string for the long's value, avoids a string builder for concatenation, avoids needing to do needless UTF-8 encoding, and eliminates a redundant use (try/finally). It should have no effect on the behavior. The same bytes will end up being written. It's just more efficient and idiomatic use of Okio.

@bcmedeiros
Copy link
Contributor Author

Is the length of the number being written 6 digits?

The bookmark just crossed the 1 million mark yesterday, so 2 days ago it was very likely that numbers were at the 6 digits. This makes a bit more sense now!

Maybe there's something going wrong with buffers in Kotlin/Native and an offset is being applied while copying bytes to the file and another memory region got copied instead?

I don't know how many people are actually using Okio on Kotlin/Native Linux, but I'd guess not many, so even if there's an issue that is platform-specific, that probably won't be as popular.

The application using this code was just a prototype as of last year, I was considering moving to Kotlin/JVM, but I got too inspired by your light bulb talk at Kotlin Conf, @JakeWharton, that I decided to stick with native Linux for a little longer!

By the way, your code can be improved a bit by changing it to:

FileSystem.SYSTEM.sink(bookmarkPath).buffer().use { s ->
b.writeDecimalLong(value)
b.writeByte('\n'.code)
}

Which avoids creation of an intermediate string for the long's value, avoids a string builder for concatenation, avoids needing to do needless UTF-8 encoding, and eliminates a redundant use (try/finally). It should have no effect on the behavior. The same bytes will end up being written. It's just more efficient and idiomatic use of Okio.

Yes, this reads much better! The buffer closes its source which I recently realised digging into Okio. I didn't know about writeDecimalLong, though, so thanks for showing me this example.

@bcmedeiros bcmedeiros changed the title Corrupted file using writeUtf8 on buffered sink Corrupted file after write using buffered sink (Kotlin/Native) Jan 30, 2025
@JakeWharton
Copy link
Collaborator

Yes, I suspect very few are using it on native Linux. Although the implementation is almost entirely POSIX-based which we share with the Apple targets, and we have a few million users of that on iOS. Of course the implementations of those POSIX APIs likely has behavior differences, although I wouldn't expect them to play a role here.

Based on what we determined about the number of bytes being written, it seems like there's some kind of pointer offset problem. We're writing the correct number of bytes as read from the segment inside the buffer, but somehow offsetting it incorrectly or maybe not pinning it properly. I took a quick look and nothing jumped out, but Jesse did all the file system code so I'm reading it all for the first time and can easily miss things.

@swankjesse
Copy link
Collaborator

The following 24 tests are currently failing on Kotlin/Native linuxArm64:

okio.NativeSystemFileSystemTest.canonicalizeDotReturnsCurrentWorkingDirectory
okio.NativeSystemFileSystemTest.listOnRelativePathWhichIsNotDotReturnsRelativePaths
okio.NativeSystemFileSystemTest.listOrNullOnRelativePathWhichIsNotDotReturnsRelativePaths
okio.ZipFileSystemGoTest.timeWinzip
okio.ZipFileSystemTest.emptyZip
okio.ZipFileSystemTest.emptyZipWithPrependedData
okio.ZipFileSystemTest.zipWithFiles
okio.ZipFileSystemTest.zipWithDeflate
okio.ZipFileSystemTest.zipWithStore
okio.ZipFileSystemTest.zipWithFileComments
okio.ZipFileSystemTest.zipWithFileModifiedDate
okio.ZipFileSystemTest.zipWithFileOutOfBoundsModifiedDate
okio.ZipFileSystemTest.zipWithDirectoryModifiedDate
okio.ZipFileSystemTest.zipWithModifiedDate
okio.ZipFileSystemTest.zipWithEmptyDirectory
okio.ZipFileSystemTest.zipWithSyntheticDirectory
okio.ZipFileSystemTest.zip64
okio.ZipFileSystemTest.zipWithArchiveComment
okio.ZipFileSystemTest.cannotReadZipWithSpanning
okio.ZipFileSystemTest.cannotReadZipWithEncryption
okio.ZipFileSystemTest.zipTooShort
okio.ZipFileSystemTest.filesOverlap
okio.ZipFileSystemTest.canonicalizationValid
okio.ZipFileSystemTest.canonicalizationInvalidThrows

test_results.txt

@swankjesse
Copy link
Collaborator

I’m using Colima to run Linux on ARM on my M1 Mac. I need to build on the host Mac because Arm64 Linux doesn’t self-host; see KT-36871.

@swankjesse
Copy link
Collaborator

Okay, all the ZipFileSystemTest tests are fixed by setting OKIO_ROOT before I run tests. Gradle does this normally, but I’m not running tests the conventional way here.

export OKIO_ROOT=/Users/jwilson/Colima/okio
./okio/build/bin/linuxArm64/debugTest/test.kexe

@swankjesse
Copy link
Collaborator

. . . . and running the tests from the right CWD gets the remaining NativeSystemFileSystemTest tests to pass. Phew.

/Users/jwilson/Colima/okio/okio

@swankjesse
Copy link
Collaborator

I wasn’t able to repro this exact situation when emulating Linux on Arm64. This test completes normally and produces the right file.

  val path = "./".toPath()

  @Test
  fun testWriteSevenByteFile() {
    set("bookmark.txt", 7_654_321L)
  }

  fun set(id: String, value: Long) {
    val bookmarkPath = path / id
    FileSystem.SYSTEM.sink(bookmarkPath).use { s ->
      s.buffer().use { b ->
        b.writeUtf8("$value\n")
      }
    }
  }
$ cat okio/bookmark.txt
7654321

@swankjesse
Copy link
Collaborator

$ xxd okio/bookmark.txt
00000000: 3736 3534 3332 310a                      7654321.

@swankjesse
Copy link
Collaborator

Also works for a 6-digit number followed by a newline, producing a 7-byte file.

$ xxd okio/bookmark.txt
00000000: 3635 3433 3231 0a                        654321.

@swankjesse
Copy link
Collaborator

Same results on x86_64.

$ xxd okio/bookmark.txt
00000000: 3635 3433 3231 0a                        654321.

I wonder what we’re doing differently.

@swankjesse
Copy link
Collaborator

Okio’s file write bottoms out in an fwrite() system call.

I wonder if something crashed, after sizing the file but before copying bytes in? Do you know if these writes succeeded?

@swankjesse
Copy link
Collaborator

@bcmedeiros did the Okio call complete normally? Any chance you’ve been able to reproduce this? So far the only thing I can think of is the underlying file system write failed, but that should have throws an exception, probably when closing the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants