[8u] RFR 8153732: Windows remote printer changes do not reflect in lookupPrintServices()

[Andrew John Hughes]

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