pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
11+ messages / 3 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-03 08:14 "vlsi (@vlsi)" <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2025-12-03 08:14 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
`Thread.inheritedAccessControlContext` inherits the caller `ProtectionDomain`, thus it effectively holds its classloader.
There's no way to "unset `inheritedAccessControlContext`".
We workaround that by going for ForkJoinPool + ManagedBlocker so it is JDK that creates a thread, thus it should have no `inheritedAccessControlContext`.
The logic is as follows:
1) ForkJoinPool task runs on CPU and it processes the queue
2) If the queue is empty, the blocking call delegates to ManagedBlocker
It fits nice with FJP: CPU tasks do not create extra threads, and blocking queue operations do not starve FJP CPU threads.
See https://stackoverflow.com/questions/78163385/does-postgres-jdbc-lazycleaner-cause-a-classloader-memo...
---
An alternative option could be going for native `java.lang.ref.Cleaner` when running with Java 9+, however, it would still be nice to have a solution for Java 8.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-03 12:50 "vlsi (@vlsi)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2025-12-03 12:50 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
For reference, I tried difftastic by @wilfred, and it produces reasonable diff:
<img width="1678" height="983" alt="diff with difftastic" src="https://github.com/user-attachments/assets/b74d120a-917a-4c1f-9c02-8b2fb4f7e9db"; />
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-04 10:44 "davecramer (@davecramer)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: davecramer (@davecramer) @ 2025-12-04 10:44 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Using claude it suggested
```
ForkJoinPool.commonPool().execute(() -> {
RefQueueBlocker<Object> blocker =
new RefQueueBlocker<>(queue, threadName, threadTtl, () -> !checkEmpty());
while (!checkEmpty()) {
try {
ref.onClean(true);
ForkJoinPool.managedBlock(blocker);
Node<?> ref = (Node<?>) blocker.drainOne();
if (ref != null) {
ref.onClean(true);
}
} catch (InterruptedException e) {
if (checkEmpty()) {
LOGGER.log(Level.FINE,
"Cleanup queue is empty, and got interrupt, will terminate the cleanup thread");
break;
}
LOGGER.log(Level.FINE, "Ignoring interrupt since the cleanup queue is non-empty");
} catch (Throwable e) {
LOGGER.log(Level.WARNING, "Unexpected exception while executing onClean", e);
}
}
});
```
as a simplification to the try catch logic
Also had this to say
5. Missing edge cases
- What happens if ForkJoinPool.commonPool() is unavailable (custom security manager)?
- No handling for RejectedExecutionException from execute()
- The threadFactory.newThread() can return null but only logs a warning
-
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-04 10:54 "davecramer (@davecramer)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: davecramer (@davecramer) @ 2025-12-04 10:54 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
What about creating a multi-release jar and have java 9+ code in there as well? I expect the number of people actually using java 8 is declining.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-04 11:51 "vlsi (@vlsi)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2025-12-04 11:51 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> What happens if ForkJoinPool.commonPool() is unavailable (custom security manager)?
Frankly, I do not think it is a real scenario.
> No handling for RejectedExecutionException from execute()
Frankly, `commonPool()` should not reject unless it shuts down.
I think we should not add workarounds for RejectedExecutionException.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-04 12:08 "davecramer (@davecramer)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: davecramer (@davecramer) @ 2025-12-04 12:08 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> > What happens if ForkJoinPool.commonPool() is unavailable (custom security manager)?
>
> Frankly, I do not think it is a real scenario.
>
> > No handling for RejectedExecutionException from execute()
>
> Frankly, `commonPool()` should not reject unless it shuts down. I think we should not add workarounds for RejectedExecutionException.
Fair enough. After giving this more thought I really do think we should consider leaving the code as is and providing a multi-release jar that targets java 9+. I would imagine most folks that are running into the memory leak issue are using Java 9 or greater
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-08 09:25 "vlsi (@vlsi)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2025-12-08 09:25 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I realized that with multi-release jar we can have the same class name for Java 8 and Java 11 implementations, and we do not need reflection in the runtime
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-08 14:37 "vlsi (@vlsi)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2025-12-08 14:37 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I think the code is good to go now
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2025-12-15 12:44 "davecramer (@davecramer)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: davecramer (@davecramer) @ 2025-12-15 12:44 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
sorry, I was away last week, Great work on this!
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2026-02-17 14:22 "philipwhiuk (@philipwhiuk)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: philipwhiuk (@philipwhiuk) @ 2026-02-17 14:22 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Switching to multi-release JAR broke my assembly build at runtime on Java 17 because it couldn't find LazyCleanerImpl until I added `<Multi-Release>true</Multi-Release>` to the assembled JAR manifest. Might be worth flagging up a bit more in the release notes (wasn't expecting a breaking change in a bugfix release).
```
Unhandled error occurred: org/postgresql/util/LazyCleanerImpl$CleanableWrapper
java.lang.NoClassDefFoundError: org/postgresql/util/LazyCleanerImpl$CleanableWrapper
at org.postgresql.util.LazyCleanerImpl.register(LazyCleanerImpl.java:65)
at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:403)
at org.postgresql.Driver.makeConnection(Driver.java:448)
at org.postgresql.Driver.connect(Driver.java:298)
at com.zaxxer.hikari.util.DriverDataSource.getConnection(DriverDataSource.java:137)
```
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext
@ 2026-02-17 14:30 "vlsi (@vlsi)" <[email protected]>
9 siblings, 0 replies; 11+ messages in thread
From: vlsi (@vlsi) @ 2026-02-17 14:30 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@philipwhiuk , could you clarify?
Do you mean you shade pgjdbc within your app?
Do you mean pgjdbc misses multi-release in jar manifest?
I just checked postgresql-42.7.10.jar, and it does seem to have `Multi-Release: true`
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2026-02-17 14:30 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-12-03 08:14 [pgjdbc/pgjdbc] PR #3886: fix: avoid memory leaks in Java <= 21 caused by Thread.inheritedAccessControlContext "vlsi (@vlsi)" <[email protected]>
2025-12-03 12:50 ` "vlsi (@vlsi)" <[email protected]>
2025-12-04 10:44 ` "davecramer (@davecramer)" <[email protected]>
2025-12-04 10:54 ` "davecramer (@davecramer)" <[email protected]>
2025-12-04 11:51 ` "vlsi (@vlsi)" <[email protected]>
2025-12-04 12:08 ` "davecramer (@davecramer)" <[email protected]>
2025-12-08 09:25 ` "vlsi (@vlsi)" <[email protected]>
2025-12-08 14:37 ` "vlsi (@vlsi)" <[email protected]>
2025-12-15 12:44 ` "davecramer (@davecramer)" <[email protected]>
2026-02-17 14:22 ` "philipwhiuk (@philipwhiuk)" <[email protected]>
2026-02-17 14:30 ` "vlsi (@vlsi)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox