[icedtea-web] PR1145: IcedTea-Web can cause ClassCircularityError

[Adam Domurad]

On Mon, 2012-08-27 at 17:51 -0400, Omair Majid wrote:
> Hi,
> 
> I think I have a fix for PR1145 now.
> 
> Eclipse shows a stack trace that looks like this:
> 
> Thread [Start] (Suspended (exception ClassCircularityError))	
> owns: JNLPClassLoader  (id=72)	
> WebStartBootImpl$1.getPermissions(CodeSource) line: not available	
> WebStartBootImpl$1(Policy).getPermissions(ProtectionDomain) line: 679	
> WebStartBootImpl$1(Policy).implies(ProtectionDomain, Permission) line: 732	
> ProtectionDomain.implies(Permission) line: 272	
> AccessControlContext.checkPermission(Permission) line: 344	
> JNLPClassLoader.getAccessControlContextForClassLoading() line: 2148	
> JNLPClassLoader.findLoadedClassAll(String) line: 1497	
> JNLPClassLoader.loadClass(String) line: 1529	
> WebStartBootImpl$1.getPermissions(CodeSource) line: not available	
> WebStartBootImpl$1(Policy).getPermissions(ProtectionDomain) line: 679	
> WebStartBootImpl$1(Policy).implies(ProtectionDomain, Permission) line: 732	
> ProtectionDomain.implies(Permission) line: 272	
> AccessControlContext.checkPermission(Permission) line: 344	
> AccessController.checkPermission(Permission) line: 555	
> JNLPSecurityManager(SecurityManager).checkPermission(Permission) line: 549	
> JNLPSecurityManager.checkPermission(Permission) line: 284	
> BootImpl$2(Thread).setContextClassLoader(ClassLoader) line: 1482	
> MainApplication.run(Object) line: 26	
> ApplicationLauncher.launchApplication(String, String[]) line: not available	
> BootImpl$2.run() line: not available	
> 
> (for some strange reason, the stack trace displayed on the command line
> leaves out the circularity between WebStartBootImpl$1.getPermissions and
> WebStartBootImpl$1.getPermissions).

Good job on finding the full stack trace! that was definitely causing a
lot of headaches.
> 
> As far as I have been able to gather, the classloader tries to verify
> the class WebStartBootImpl$1 (a subclass of java.security.Policy). This
> class refers to java/security/PermissionCollection and so the
> classloader tries to 'load' that too. The first step of 'loading' a
> class (as implemented in ClassLoader.loadClass) is to check if the class
> has been loaded. This is where we run into the problem: we try to
> execute a doPrivileged block and end up querying the Policy object
> (WebStartBootImpl$1) again. The ClassLoader still hasn't figured out if
> java/security/PermissionCollection exists and fails when it encounters
> the class again.
> 
> A simple enough fix, in my opinion, is to remove the doPrivileged block.
> The javadocs for ClassLoader.findLoadedClass do not mention any
> permissions required to call the method nor do they mention that the
> method can throw a SecurityException. The native code that implements
> findLoadedClass does not have any security checks either. This leaves me
> to assume that the doProvileged block is not needed at all here. This
> will skip querying the Policy object and break the circularity.
> 
> The attached patch accomplishes this.
> 
> Thoughts? Okay for 1.3 and HEAD?
> 
> Thanks,
> Omair

Do you also want to incorporate your first idea into HEAD ? (I
admittedly skimmed the patch but you said it brought it closer to
classloader specs). 

Anyway, this checks out with me. OK for 1.3/HEAD. 

Thanks!
- Adam