On 04/06/2019 16:08, Zhengyu Gu wrote: > Ping! > > Thanks, > > -Zhengyu > > On 5/28/19 9:03 AM, Zhengyu Gu wrote: >> Hi, >> >> Could you please review this 8u backport? After JDK-8055705 backport, >> the original patch still does not apply cleanly, due to missing >> following Thread API in JDK8u. >> >> public Thread(ThreadGroup group, >> Runnable target, >> String name, >> long stackSize, >> boolean inheritThreadLocals) >> >> The original patch has inheritThreadLocals = false, but replacement API >> >> public Thread(ThreadGroup group, >> Runnable target, >> String name, >> long stackSize) >> has it default to true. >> >> Could AWT folks take a look? >> >> >> JDK9 bug: https://bugs.openjdk.java.net/browse/JDK-8153732 >> Original patch: http://hg.openjdk.java.net/jdk/client/rev/732a3b600098 >> >> >> 8u Backport: http://cr.openjdk.java.net/~zgu/JDK-8153732_8u/webrev.01/ >> >> Test: >> manual test: RemotePrinterStatusRefresh >> >> Thanks, >> >> -Zhengyu >> >> Thanks, >> >> -Zhengyu >>
There's quite a convoluted history to this, which unfortunately involves
a number of bugs that aren't visible.
8048210: More Enhancements to InnocuousThread and friends introduces the
class sun.misc.ManagedLocalsThread
8027771: Enhance thread contexts makes the change:
- PrinterChangeListener thr = new PrinterChangeListener();
+ Thread thr;
+ if (System.getSecurityManager() == null) {
+ thr = new Thread(new PrinterChangeListener());
+ } else {
+ thr = new ManagedLocalsThread(new PrinterChangeListener());
+ }
and
- class PrinterChangeListener extends Thread {
+ class PrinterChangeListener implements Runnable {
8080405: Exception in thread "AWT-EventQueue-1"
java.security.AccessControlException simplifies this to:
- Thread thr;
- if (System.getSecurityManager() == null) {
- thr = new Thread(new PrinterChangeListener());
- } else {
- thr = new ManagedLocalsThread(new PrinterChangeListener());
- }
+ Thread thr = new ManagedLocalsThread(new
PrinterChangeListener());
8147544: Remove sun.misc.ManagedLocalsThread from java.desktop changes
this to:
- Thread thr = new ManagedLocalsThread(new
PrinterChangeListener());
+ Thread thr = new Thread(null, new PrinterChangeListener(),
+ "PrinterListener", 0, false);
As such, I don't think the change unique to your patch:
- // start the printer listener thread
- PrinterChangeListener thr = new PrinterChangeListener();
+ // start the local printer listener thread
+ Thread thr = new Thread(null, new PrinterChangeListener(),
+ "PrinterListener", 0);
is appropriate. Rather, I would suggest altering remThr to make it
consistent with the existing 8u code:
+ // start the remote printer listener thread
+ Thread remThr = new RemotePrinterChangeListener(),
...
- class RemotePrinterChangeListener implements Runnable {
+ class RemotePrinterChangeListener extends Thread {
Maybe JDK-8027771 & JDK-8080405 should be backported at some later date,
but it's hard to tell when there is no information on why they are being
applied.
--
Andrew :)
Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew
signature.asc
Description: OpenPGP digital signature
