Fix file descriptor leak in FileIO instrumentation (#2248)

Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Co-authored-by: Alexander Dinauer <alexander.dinauer@sentry.io> Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
getsentry · Sep 19, 2022 · 9cd49cd · 9cd49cd
1 parent 429f878
commit 9cd49cd
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 - Fixed AbstractMethodError when getting Lifecycle ([#2228](https://github.com/getsentry/sentry-java/pull/2228))
 - Missing unit fields for Android measurements ([#2204](https://github.com/getsentry/sentry-java/pull/2204))
 - Avoid sending empty profiles ([#2232](https://github.com/getsentry/sentry-java/pull/2232))
+- Fix file descriptor leak in FileIO instrumentation ([#2248](https://github.com/getsentry/sentry-java/pull/2248))
 
 ## 6.4.1
 

diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileInputStream.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileInputStream.java
@@ -54,7 +54,7 @@ private SentryFileInputStream(
 
   private SentryFileInputStream(final @NotNull FileInputStreamInitData data)
       throws FileNotFoundException {
-    super(data.file);
+    super(getFileDescriptor(data.delegate));
     spanManager = new FileIOSpanManager(data.span, data.file, data.isSendDefaultPii);
     delegate = data.delegate;
   }
@@ -114,6 +114,15 @@ public void close() throws IOException {
     spanManager.finish(delegate);
   }
 
+  private static FileDescriptor getFileDescriptor(final @NotNull FileInputStream stream)
+      throws FileNotFoundException {
+    try {
+      return stream.getFD();
+    } catch (IOException error) {
+      throw new FileNotFoundException("No file descriptor");
+    }
+  }
+
   public static final class Factory {
     public static FileInputStream create(
         final @NotNull FileInputStream delegate, final @Nullable String name)

diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java b/sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java
@@ -59,7 +59,7 @@ private SentryFileOutputStream(
 
   private SentryFileOutputStream(final @NotNull FileOutputStreamInitData data)
       throws FileNotFoundException {
-    super(data.file, data.append);
+    super(getFileDescriptor(data.delegate));
     spanManager = new FileIOSpanManager(data.span, data.file, data.isSendDefaultPii);
     delegate = data.delegate;
   }
@@ -120,6 +120,15 @@ public void close() throws IOException {
     spanManager.finish(delegate);
   }
 
+  private static FileDescriptor getFileDescriptor(final @NotNull FileOutputStream stream)
+      throws FileNotFoundException {
+    try {
+      return stream.getFD();
+    } catch (IOException error) {
+      throw new FileNotFoundException("No file descriptor");
+    }
+  }
+
   public static final class Factory {
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @Nullable String name)

diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileInputStreamTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileInputStreamTest.kt
@@ -16,6 +16,7 @@ import java.io.FileInputStream
 import java.io.IOException
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 
 class SentryFileInputStreamTest {
 
@@ -96,6 +97,13 @@ class SentryFileInputStreamTest {
         assertEquals(fileIOSpan.status, SpanStatus.OK)
     }
 
+    @Test
+    fun `when stream is closed, releases file descriptor`() {
+        val fis = fixture.getSut(tmpFile)
+        fis.use { it.readAllBytes() }
+        assertFalse(fis.fd.valid())
+    }
+
     @Test
     fun `read one byte`() {
         fixture.getSut(tmpFile).use { it.read() }

diff --git a/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt b/sentry/src/test/java/io/sentry/instrumentation/file/SentryFileOutputStreamTest.kt
@@ -12,6 +12,7 @@ import org.junit.rules.TemporaryFolder
 import java.io.File
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 
 class SentryFileOutputStreamTest {
     class Fixture {
@@ -68,6 +69,13 @@ class SentryFileOutputStreamTest {
         assertEquals(fileIOSpan.status, SpanStatus.OK)
     }
 
+    @Test
+    fun `when stream is closed file descriptor is also closed`() {
+        val fos = fixture.getSut(tmpFile)
+        fos.use { it.write("hello".toByteArray()) }
+        assertFalse(fos.fd.valid())
+    }
+
     @Test
     fun `write one byte`() {
         fixture.getSut(tmpFile).use { it.write(29) }