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

Try to use java 9 Cleaner #3090

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion build-logic/build-parameters/build.gradle.kts
Expand Up @@ -17,7 +17,7 @@ buildParameters {
description.set("Collect test coverage")
}
integer("targetJavaVersion") {
defaultValue.set(8)
defaultValue.set(9)
mandatory.set(true)
description.set("Java version for source and target compatibility")
}
Expand Down
17 changes: 6 additions & 11 deletions pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
Expand Up @@ -41,7 +41,6 @@
import org.postgresql.util.DriverInfo;
import org.postgresql.util.GT;
import org.postgresql.util.HostSpec;
import org.postgresql.util.LazyCleaner;
import org.postgresql.util.LruCache;
import org.postgresql.util.PGBinaryObject;
import org.postgresql.util.PGInterval;
Expand All @@ -61,6 +60,7 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.ref.Cleaner;
import java.security.Permission;
import java.sql.Array;
import java.sql.Blob;
Expand Down Expand Up @@ -105,7 +105,7 @@ public class PgConnection implements BaseConnection {
private static final Set<Integer> SUPPORTED_BINARY_OIDS = getSupportedBinaryOids();
private static final SQLPermission SQL_PERMISSION_ABORT = new SQLPermission("callAbort");
private static final SQLPermission SQL_PERMISSION_NETWORK_TIMEOUT = new SQLPermission("setNetworkTimeout");

private static final Cleaner cleaner = Cleaner.create();
private static final @Nullable MethodHandle SYSTEM_GET_SECURITY_MANAGER;
private static final @Nullable MethodHandle SECURITY_MANAGER_CHECK_PERMISSION;

Expand Down Expand Up @@ -217,7 +217,7 @@ private enum ReadOnlyBehavior {

private final @Nullable String xmlFactoryFactoryClass;
private @Nullable PGXmlFactoryFactory xmlFactoryFactory;
private final LazyCleaner.Cleanable<IOException> cleanable;
private final Cleaner.Cleanable cleanable;

final CachedQuery borrowQuery(String sql) throws SQLException {
return queryExecutor.borrowQuery(sql);
Expand Down Expand Up @@ -251,6 +251,7 @@ public void setFlushCacheOnDeallocate(boolean flushCacheOnDeallocate) {

//
// Ctor.
// Ctor.
//
@SuppressWarnings({"method.invocation"})
public PgConnection(HostSpec[] hostSpecs,
Expand Down Expand Up @@ -383,7 +384,7 @@ public PgConnection(HostSpec[] hostSpecs,
replicationConnection = PGProperty.REPLICATION.getOrDefault(info) != null;

xmlFactoryFactoryClass = PGProperty.XML_FACTORY_FACTORY.getOrDefault(info);
cleanable = LazyCleaner.getInstance().register(leakHandle, finalizeAction);
cleanable = cleaner.register(leakHandle, finalizeAction);
}

private static ReadOnlyBehavior getReadOnlyBehavior(@Nullable String property) {
Expand Down Expand Up @@ -864,13 +865,7 @@ public void close() throws SQLException {
return;
}
openStackTrace = null;
try {
cleanable.clean();
} catch (IOException e) {
throw new PSQLException(
GT.tr("Unable to close connection properly"),
PSQLState.UNKNOWN_STATE, e);
}
cleanable.clean();
}

@Override
Expand Down
Expand Up @@ -6,15 +6,11 @@
package org.postgresql.jdbc;

import org.postgresql.Driver;
import org.postgresql.util.GT;
import org.postgresql.util.LazyCleaner;

import org.checkerframework.checker.nullness.qual.Nullable;

import java.io.Closeable;
import java.io.IOException;
import java.util.Timer;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
Expand All @@ -26,7 +22,7 @@
* <li>Release shared timer registration</li>
* </ul>
*/
class PgConnectionCleaningAction implements LazyCleaner.CleaningAction<IOException> {
class PgConnectionCleaningAction implements Runnable {
private static final Logger LOGGER = Logger.getLogger(PgConnection.class.getName());

private final ResourceLock lock;
Expand Down Expand Up @@ -80,12 +76,13 @@ public void purgeTimerTasks() {
}

@Override
public void onClean(boolean leak) throws IOException {
if (leak && openStackTrace != null) {
LOGGER.log(Level.WARNING, GT.tr("Leak detected: Connection.close() was not called"), openStackTrace);
}
public void run() {
openStackTrace = null;
releaseTimer();
queryExecutorCloseAction.close();
try {
queryExecutorCloseAction.close();
} catch ( Exception ex ) {
//ignore
}
}
}
17 changes: 10 additions & 7 deletions pgjdbc/src/main/java/org/postgresql/util/SharedTimer.java
Expand Up @@ -9,35 +9,37 @@

import org.checkerframework.checker.nullness.qual.Nullable;

import java.lang.ref.Cleaner;
import java.util.Timer;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Level;
import java.util.logging.Logger;

public class SharedTimer {
static class TimerCleanup implements LazyCleaner.CleaningAction<RuntimeException> {
static class TimerCleanup implements Runnable {
private final Timer timer;

TimerCleanup(Timer timer) {
this.timer = timer;
}

@Override
public void onClean(boolean leak) throws RuntimeException {
public void run() {
timer.cancel();
}
}

// Incremented for each Timer created, this allows each to have a unique Timer name
private static final AtomicInteger timerCount = new AtomicInteger(0);
private static final Cleaner cleaner = Cleaner.create();

private static final Logger LOGGER = Logger.getLogger(SharedTimer.class.getName());
private volatile @Nullable Timer timer;
private final AtomicInteger refCount = new AtomicInteger(0);
private final ResourceLock lock = new ResourceLock();
private LazyCleaner.@Nullable Cleanable<RuntimeException> timerCleanup;
private Cleaner.Cleanable cleanable;

public SharedTimer() {

Check failure on line 42 in pgjdbc/src/main/java/org/postgresql/util/SharedTimer.java

View workflow job for this annotation

GitHub Actions / CheckerFramework

[Task :postgresql:compileJava] [initialization.fields.uninitialized] the constructor does not initialize fields: cleanable public SharedTimer() { ^
}

public int getRefCount() {
Expand All @@ -62,7 +64,8 @@
Thread.currentThread().setContextClassLoader(null);

this.timer = timer = new Timer("PostgreSQL-JDBC-SharedTimer-" + index, true);
this.timerCleanup = LazyCleaner.getInstance().register(refCount, new TimerCleanup(timer));
cleanable = cleaner.register(refCount, new TimerCleanup(timer));

} finally {
Thread.currentThread().setContextClassLoader(prevContextCL);
}
Expand All @@ -81,10 +84,10 @@
} else if (count == 0) {
// This is the last usage of the Timer so cancel it so it's resources can be release.
LOGGER.log(Level.FINEST, "No outstanding references to shared Timer, will cancel and close it");
if (timerCleanup != null) {
timerCleanup.clean();
if (cleanable != null) {
cleanable.clean();
timer = null;
timerCleanup = null;
cleanable = null;
}
} else {
// Should not get here under normal circumstance, probably a bug in app code.
Expand Down
11 changes: 7 additions & 4 deletions pgjdbc/src/main/java/org/postgresql/util/StreamWrapper.java
Expand Up @@ -16,6 +16,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.lang.ref.Cleaner;
import java.nio.file.Files;
import java.nio.file.Path;

Expand All @@ -26,9 +27,12 @@
*/
public final class StreamWrapper implements Closeable {

private static final Cleaner cleaner = Cleaner.create();

private static final int MAX_MEMORY_BUFFER_BYTES = 51200;

private static final String TEMP_FILE_PREFIX = "postgres-pgjdbc-stream";
private Cleaner.Cleanable cleanable;

public StreamWrapper(byte[] data, int offset, int length) {
this.stream = null;
Expand Down Expand Up @@ -74,7 +78,7 @@ public StreamWrapper(InputStream stream) throws PSQLException {
this.stream = null; // The stream is opened on demand
TempFileHolder tempFileHolder = new TempFileHolder(tempFile);
this.tempFileHolder = tempFileHolder;
cleaner = LazyCleaner.getInstance().register(leakHandle, tempFileHolder);
cleanable = cleaner.register(leakHandle, tempFileHolder);
} else {
this.rawData = rawData;
this.stream = null;
Expand All @@ -101,8 +105,8 @@ public InputStream getStream() throws IOException {

@Override
public void close() throws IOException {
if (cleaner != null) {
cleaner.clean();
if (cleanable != null) {
cleanable.clean();
}
}

Expand Down Expand Up @@ -142,7 +146,6 @@ private static int copyStream(InputStream inputStream, OutputStream outputStream
private final @Nullable InputStream stream;
private @Nullable TempFileHolder tempFileHolder;
private final Object leakHandle = new Object();
private LazyCleaner.@Nullable Cleanable<IOException> cleaner;
private final byte @Nullable [] rawData;
private final int offset;
private final int length;
Expand Down
14 changes: 7 additions & 7 deletions pgjdbc/src/main/java/org/postgresql/util/TempFileHolder.java
Expand Up @@ -13,14 +13,13 @@
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* The action deletes temporary file in case the user submits a large input stream,
* and then abandons the statement.
*/
class TempFileHolder implements LazyCleaner.CleaningAction<IOException> {
class TempFileHolder implements Runnable {

private static final Logger LOGGER = Logger.getLogger(StreamWrapper.class.getName());
private @Nullable InputStream stream;
Expand All @@ -40,18 +39,19 @@ public InputStream getStream() throws IOException {
}

@Override
public void onClean(boolean leak) throws IOException {
if (leak) {
LOGGER.log(Level.WARNING, GT.tr("StreamWrapper leak detected StreamWrapper.close() was not called. "));
}
public void run() {
Path tempFile = this.tempFile;
if (tempFile != null) {
tempFile.toFile().delete();
this.tempFile = null;
}
InputStream stream = this.stream;
if (stream != null) {
stream.close();
try {
stream.close();
} catch (IOException e) {
throw new RuntimeException(e);
}
this.stream = null;
}
}
Expand Down