Can be a such change of behavior acceptable for CP (cumulative patch) release?

There should not be any backwards compatibility issues.

The class being loaded is in the Resteasy module.
Before the fix, it was loaded indirectly from the Resteasy module, as a dependency of the TCCL.
With the fix, it is loaded directly from the Resteasy module.

There are only 2 use cases that will change behavior:
1.  TCCL does not have a dependency on the Resteasy module.
  This is the use case this BZ is intended to correct.
2.  TCCL contains a different version of Resteasy.
  The previous behavior was wrong, and likely didn't work at all.

Thank you Dennis for explanation. I agree with you and I'm going to set QA ACK+.

Is there a reproducer of CNFE or more detailed info how to reproduce the issue? It might be the quickest way how to verify the fix without too much additional coding on our side. Thank you.

Reported to upstream: https://issues.jboss.org/browse/RESTEASY-901

It's targeted at 2.3.7.Final

Fixed in RESTEasy 2.3.7.Final

class org.jboss.resteasy.logging.Logger from resteasy-jaxrs-2.3.7.Final-redhat-2 is still using TCCL in its static initialization block, which leads to login module failure in case when TCCL is not set (is null)

   private static boolean classInClasspath(String className)
   {
      try
      {
         return Thread.currentThread().getContextClassLoader().loadClass(className) != null;
      }
      catch (ClassNotFoundException e)
      {
         return false;
      }

   }

   static
   {
      LoggerType type = LoggerType.JUL;

      if (classInClasspath("org.apache.log4j.Logger"))
      {
         type = LoggerType.LOG4J;
      }
      else if (classInClasspath("org.slf4j.Logger"))
      {
         type = LoggerType.SLF4J;
      }

      setLoggerType(type);
   }

Fixed in 2.3.8:

   static
   {
      LoggerType type = LoggerType.JUL;

      if (classInClasspath("org.apache.log4j.Logger"))
      {
         type = LoggerType.LOG4J;
      }
      else if (classInClasspath("org.slf4j.Logger"))
      {
         type = LoggerType.SLF4J;
      }

      setLoggerType(type);
   }


...

   public static void setLoggerType(LoggerType loggerType)
   {
      try
      {
         Class loggerClass = null;
         if (loggerType == LoggerType.JUL)
         {
            loggerClass = Logger.class.getClassLoader().loadClass("org.jboss.resteasy.logging.impl.JULLogger"); <--- defining classloader used
         }
         else if (loggerType == LoggerType.LOG4J)
         {
            loggerClass = Logger.class.getClassLoader().loadClass("org.jboss.resteasy.logging.impl.Log4jLogger"); <--- defining classloader used
         }
         else if (loggerType == LoggerType.SLF4J)
         {
            loggerClass = Logger.class.getClassLoader().loadClass("org.jboss.resteasy.logging.impl.Slf4jLogger"); <--- defining classloader used
         }
         if (loggerClass == null)
            throw new RuntimeException("Could not match up an implementation for LoggerType: " + loggerType);
         loggerConstructor = loggerClass.getDeclaredConstructor(String.class);
      }
      catch (ClassNotFoundException e)
      {
         throw new RuntimeException(e);
      }
      catch (NoSuchMethodException e)
      {
         throw new RuntimeException(e);
      }

   }

btw, I saw this is also fixed in 2.3.7.1. Because the fix is in 'setLoggerType()' method, not the 'classInClasspath()' method. The classInClasspath shouldn't be changed as trying to find the correct logger framework is expected.

Hi Weinan,

I think Petr meant something else in his comment 7. In the fix we use Logger classloader instead of TCCL classloader: 

Thread.currentThread().getContextClassLoader() -> Logger.class.getClassLoader()

Which is ok. The problem is in classInClasspath() method we still use TCCL to check whether specified class is in the class path. Shouldn't we use again Logger classloader?

(In reply to Katerina Novotna from comment #10)
> Hi Weinan,
> 
> I think Petr meant something else in his comment 7. In the fix we use Logger
> classloader instead of TCCL classloader: 
> 
> Thread.currentThread().getContextClassLoader() ->
> Logger.class.getClassLoader()
> 
> Which is ok. The problem is in classInClasspath() method we still use TCCL
> to check whether specified class is in the class path. Shouldn't we use
> again Logger classloader?

As I understand we use TCCL to find the proper Logger in WildFly/EAP environment:

...
classInClasspath("org.apache.log4j.Logger") <-- TCCL used to find logger framework in env
...

And then we use class defining class loader to load RESTEasy log module:

loggerClass = Logger.class.getClassLoader().loadClass("org.jboss.resteasy.logging.impl.Slf4jLogger"); <--- defining classloader used to load relative resteasy logger.

Ron, could you please help to verify this?

Verification postponed to ER10, because we are waiting for information.

I thought about it second time and I see it like this:

If Thread.currentThread().getContextClassLoader() in classInClasspath method returns null then calling loadClass() on null would cause NullPointerException, therefore using Logger.class.getClassLoader().loadClass() will better way to handle this.

For example:

ClassLoader cl = Thread.currentThread().getContextClassLoader();
if (cl == null) cl = Logger.class.getClassLoader(); // fallback
Class clazz = cl.loadClass(name);

The issue in #7 is not related to this BZ (loading the RestEasy classes from RestEasy's classloader instead of the TCCL).
That part is currently working as designed (loading the logger impl from the TCCL)

If you believe #13 is an issue, that should be a separate BZ.

Thank you Dennis for your comment.

I will then close this bz.

Verified in EAP 6.3.0.ER10.