RFR: regex changes -- sun.security.util.Debug issue

[Xueming Shen]

Sean,

Given the "debug" is a private field and it is used only for debug != null
in toString() -> seeAllp(), can the following change be a valid update?
with the assumption that

(1) the toString() is really not performance critical here, as we are changing
     from
         debug != null
     to

    public static boolean isOn(String option)
     {
         if (args == null)
             return false;
         else {
             if (args.indexOf("all") != -1)
                 return true;
             else
                 return (args.indexOf(option) != -1);
         }
     }


(2) the "debug" field is not being accessed via reflect


---------------------------------------------------------------------------------------
src/java.base/share/classes/java/security/ProtectionDomain.java

@@ -137,12 +137,10 @@
      /*
       * An object used as a key when the ProtectionDomain is stored in a Map.
       */
      final Key key = new Key();

-    private static final Debug debug = Debug.getInstance("domain");
-
      /**
       * Creates a new ProtectionDomain with the given CodeSource and
       * Permissions. If the permissions object is not null, then
       *  {@code setReadOnly())} will be called on the passed in
       * Permissions object. The only permissions granted to this domain

@@ -357,11 +355,11 @@
          SecurityManager sm = System.getSecurityManager();

          if (sm == null) {
              return true;
          } else {
-            if (debug != null) {
+            if (Debug.isOn("domain")) {
                  if (sm.getClass().getClassLoader() == null &&
                      Policy.getPolicyNoCheck().getClass().getClassLoader()
                                                                  == null) {
                      return true;
                  }
--------------------------------------------------------------------------------------------------


On 05/10/2016 10:23 AM, Sean Mullan wrote:
>
>
> On 5/10/16 1:30 AM, Alan Bateman wrote:
>> On 10/05/2016 06:36, Xueming Shen wrote:
>>> Hi,
>>>
>>> While testing for the attached regex changes, a fatal vm init error
>>> was triggered for test
>>> case with -Djava.security.debug=xyz turned on, as showed in following
>>> stacktrace.
>>>
>>> It appears sun.security.util.Debug is being initialized even before
>>> the lambda is ready
>>> for use, and unfortunately it uses j.u.regex (for its args parsing),
>>> which is being migrated
>>> to use lambda function in the proposed regex change.
>>>
>>> Since Debug is the only class now triggers j.u.regex -> lambda during
>>> initialization, it
>>> is suggested to update/rewrite the related method in Debug to NOT use
>>> j.u.regex to
>>> solve/workaround this specific initialization issue.
>> This is always tricky but I wonder how far we could get by initializing
>> ProtectionDomain.debug and SecureClassLoader.debug lazily. That is, move
>> them to a holder class where I wouldn't expect they will be initialized
>> until much later, esp. as the security manager won't be set until later
>> in the initialization (or init phase 3 as we've come to name it).
>
> Actually, ProtectionDomain already does something like this. It only writes to debug if the policy has been set. See toString():
>
>        PermissionCollection pc = Policy.isSet() && seeAllp() ?
>                                       mergePermissions():
>                                       getPermissions();
>
> So, a simpler fix is to extend that concept and not initialize the debug field until Policy.isSet is true. This should resolve this bootstrapping issue.
>
> --Sean