Skip to content

Commit

Permalink
Replace finalizer of NativeFileOutputStream.
Browse files Browse the repository at this point in the history
This removes the last deprecated object finalizer from Bazel.

I created a single java.lang.ref.Cleaner instance for multiple users in Bazel.
  • Loading branch information
benjaminp committed Feb 11, 2025
1 parent d7c71fa commit 3f0720f
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/shell/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ java_library(
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/jni",
"//src/main/java/com/google/devtools/build/lib/util:cleaner",
"//src/main/java/com/google/devtools/build/lib/util:describable_execution_unit",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.shell;

import static com.google.devtools.build.lib.util.BazelCleaner.CLEANER;

import com.google.common.base.Throwables;
import com.google.devtools.build.lib.windows.WindowsProcesses;
import java.io.IOException;
Expand All @@ -29,8 +31,6 @@

/** A Windows subprocess backed by a native object. */
public class WindowsSubprocess implements Subprocess {
private static final Cleaner cleaner = Cleaner.create();

// For debugging purposes.
private String commandLine;

Expand Down Expand Up @@ -200,7 +200,7 @@ public void run() {
stderrRedirected
? null
: new ProcessInputStream(WindowsProcesses.getStderr(nativeProcess)));
cleanable = cleaner.register(this, nativeState);
cleanable = CLEANER.register(this, nativeState);
// As per the spec of Command, we should only apply timeouts that are > 0.
this.timeoutMillis = timeoutMillis <= 0 ? -1 : timeoutMillis;
// Every Windows process we start consumes a thread here. This is suboptimal, but seems to be
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/unix/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/jni",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:blocker",
"//src/main/java/com/google/devtools/build/lib/util:cleaner",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
"//src/main/java/com/google/devtools/build/lib/vfs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.unix;

import static com.google.devtools.build.lib.util.BazelCleaner.CLEANER;

import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
Expand All @@ -35,6 +37,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UncheckedIOException;
import java.lang.ref.Cleaner;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -489,33 +493,45 @@ protected OutputStream createFileOutputStream(PathFragment path, boolean append,
}
}

private static class NativeFileOutputStream extends OutputStream {
private final int fd;
private boolean closed = false;
private static final class FdHolder implements Runnable {
private int fd;

NativeFileOutputStream(int fd) {
private FdHolder(int fd) {
this.fd = fd;
}

@Override
protected void finalize() throws Throwable {
close();
super.finalize();
public void run() {
try {
NativePosixFiles.close(fd, this);
} catch (IOException e) {
throw new UncheckedIOException(e);
} finally {
fd = -1;
}
}
}

private static class NativeFileOutputStream extends OutputStream {
private final FdHolder fdHolder;
private final Cleaner.Cleanable cleanable;

private NativeFileOutputStream(int fd) {
this.fdHolder = new FdHolder(fd);
this.cleanable = CLEANER.register(this, fdHolder);
}

@Override
public synchronized void close() throws IOException {
if (!closed) {
public void close() throws IOException {
var comp = Blocker.begin();
try {
NativePosixFiles.close(fd, this);
closed = true;
cleanable.clean();
} catch (UncheckedIOException e) {
throw e.getCause();
} finally {
Blocker.end(comp);
}
}
super.close();
}

@Override
public void write(int b) throws IOException {
Expand All @@ -528,10 +544,9 @@ public void write(byte[] b) throws IOException {
}

@Override
@SuppressWarnings(
"UnsafeFinalization") // Finalizer invokes close; close and write are synchronized.
public synchronized void write(byte[] b, int off, int len) throws IOException {
if (closed) {
public void write(byte[] b, int off, int len) throws IOException {
var fd = fdHolder.fd;
if (fd < 0) {
throw new IOException("attempt to write to a closed Outputstream backed by a native file");
}
var comp = Blocker.begin();
Expand All @@ -546,13 +561,13 @@ public synchronized void write(byte[] b, int off, int len) throws IOException {
private static final class ProfiledNativeFileOutputStream extends NativeFileOutputStream {
private final String name;

public ProfiledNativeFileOutputStream(int fd, String name) {
private ProfiledNativeFileOutputStream(int fd, String name) {
super(fd);
this.name = name;
}

@Override
public synchronized void write(byte[] b, int off, int len) throws IOException {
public void write(byte[] b, int off, int len) throws IOException {
long startTime = Profiler.nanoTimeMaybe();
try {
super.write(b, off, len);
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/google/devtools/build/lib/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -462,3 +462,8 @@ java_library(
"//third_party:jsr305",
],
)

java_library(
name = "cleaner",
srcs = ["BazelCleaner.java"],
)
10 changes: 10 additions & 0 deletions src/main/java/com/google/devtools/build/lib/util/BazelCleaner.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.google.devtools.build.lib.util;

import java.lang.ref.Cleaner;

/** Common {@link Cleaner} for Bazel. */
public final class BazelCleaner {
public static final Cleaner CLEANER = Cleaner.create();

private BazelCleaner() {}
}

0 comments on commit 3f0720f

Please sign in to comment.