RFR: 8150173: JAXBContext.newInstance causes PrivilegedActionException when createContext's declared in absract class extended by discovered JAXB implementation

[Georgiy Rakov]

Hi Daniel,

if I understand correctly JAXBContextWithSubclassedFactory is supposed 
to capture the idea of Test9 but it doesn't. FactoryBase shouldn't 
implement JAXBContextFactory but Factory should:

     public static class FactoryBase {
         public JAXBContext createContext(...) ...

         public JAXBContext createContext(...) ...
     }

     public static class Factory extends FactoryBase implements 
JAXBContextFactory {    }

This is a quite a corner case of methods overriding when method is 
overridden by the base class.

JAXBContextWithLegacyFactory looks good to me.

BTW the approach with Factory1 and Factory2 used in 
JAXBContextWithLegacyFactory seems be useful in 
JAXBContextWithSubclassedFactory and JAXBContextWithAbstractFactory too.

It also seems that some refactoring could be applied by extracting main, 
test, JAXBContextImpl and tmp members from JAXBContextWithLegacyFactory 
to be used in all three classes..... probably by extracting superclass 
or method object refactoring.

Thank you,
Georgiy.

On 22.06.2016 18:09, Daniel Fuchs wrote:
> Hi Georgiy,
>
> I added the (reworked) promised testcases.
> I believe I have captured the intent of what you suggested.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8150173/webrev.02/
>
> best regards,
>
> -- daniel
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8150173/webrev.02/
> On 22/06/16 11:55, Georgiy Rakov wrote:
>> This is about backward compatibility, but anyway would it be good to add
>> to tests static method case (also please see the attached file):
>>
>>     public static class FactoryBase {
>>         public static JAXBContext createContext(Class[]
>> classesToBeBound, Map<String, Object> properties) throws JAXBException {
>>             return tmp;
>>         }
>>
>>         public static JAXBContext createContext(String contextPath,
>> ClassLoader classLoader, Map<String, Object> properties)
>>                 throws JAXBException {
>>             return tmp;
>>         }
>>     }
>>
>>     public static class Factory extends FactoryBase {
>>     }
>>
>> Now it works fine but it might be useful as a test anyway.
>>
>> It's a valid case according to spec which reads (please see point 2):
>>
>>  Once the provider factory class is discovered, context creation is
>> delegated to one of its createContext(...) methods. For backward
>> compatibility reasons, there are two ways how to implement provider
>> factory class:
>>
>>     ...
>>
>>     2. the class is not implementation of interface above and then it is
>> mandated to implement the following static method signatures:
>>
>>      public static JAXBContext createContext(
>>                                           String contextPath,
>>                                           ClassLoader classLoader,
>> Map<String,Object> properties ) throws JAXBException
>>
>>      public static JAXBContext createContext(
>>                                           Class[] classes,
>> Map<String,Object> properties ) throws JAXBException
>>
>>
>>     In this scenario, appropriate static method is used instead of
>> instance method. This approach is incompatible with ServiceLoader so it
>> can't be used with step 3.
>>
>> Thanks,
>> Georgiy.
>>
>> On 21.06.2016 20:39, Daniel Fuchs wrote:
>>> Hi,
>>>
>>> Please find below a somewhat trivial patch for
>>>
>>> 8150173: JAXBContext.newInstance causes PrivilegedActionException
>>>          when createContext's declared in absract class extended
>>>          by discovered JAXB implementation
>>> https://bugs.openjdk.java.net/browse/JDK-8150173
>>>
>>> Patch:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8150173/webrev.00
>>>
>>> This is an oversight that was introduced with JDK-8145104.
>>>
>>> The issue is simply that newInstance() must be invoked on
>>> the concrete class, not on the class that defines the
>>> createContext method.
>>>
>>> best regards,
>>>
>>> -- daniel
>>
>