osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

Import packages in org.osgi.test.cases.provisioning is wrong #24

Closed bjhargrave closed 18 years ago

bjhargrave commented 19 years ago

Original bug ID: BZ#29 From: Pavlin Dobrev <p.dobrev@prosyst.com> Reported version: R4

bjhargrave commented 19 years ago

Comment author: Pavlin Dobrev <p.dobrev@prosyst.com>

The org.osgi.test.cases.provisioning does not import javax.servlet package in the manifest file. It imports only javax.servlet.http.* but the HttpServlet depends on javax.servlet package that is missing in the import.

I think that in the RI javax.servlet.http.* doesn't set javax.servlet as an implied package.

Maybe the problem is in the BTOOL. Can you check this?

(I use with http://membercvs.osgi.org/sp-r4/ - osgi.ri-tck.zip)

bjhargrave commented 19 years ago

Comment author: @bjhargrave

Pavlin Dobrev <p.dobrev@ prosyst.com> wrote on 2005-10-03 11:07:27 AM:

Hi Peter,

There is no connection on source level but there is a dependency at bytecode level:

RshServlet, line 107: DataOutputStream out = new DataOutputStream( rsp.getOutputStream() );

ProvisioningTestCase, line 80: registerServlet("/test/rsh", new RshServlet(),

The referenced are javax.servlet.ServletOutputStream and javax.servlet.Servlet and the JVM try to load it using Class loader of the test.

-Pavlin

PK> 1. Why would it need to import javax.servlet? It will load PK> javax.servlet.http.HttpServlet, and that class refers to PK> javax.servlet. So it will use the class loader of PK> javax.servlet.http.HttpServlet class. None of the Java sources PK> import javax.servlet so why should it be imported?

PK> 2. That said. In the current build, the javax.servlet IS for some PK> reason included in the test case, which I do not really understand :-(

PK> Kind regards,

PK> Peter Kriens

It would seem that javax.servlet package should be imported by the test case since the byte codes reference it. Perhaps this did not surface since the ant script to run the test cases turns the verifier off (-Xverify:none). The verifier may have attempted to load the servlet classes since they are references by the byte codes.

This does seem to be a btool problem.

So the main question here is whether this problem invalidates the test case for R4?

bjhargrave commented 19 years ago

Comment author: @pkriens

Btool correctly generates the import statement in my build. However, the build in the repository was done by BJ. The class files in this build do not refer to the javax/servlet package, classes in my build do.

BJ, are you still using the IBM compiler?

bjhargrave commented 19 years ago

Comment author: @pkriens

I looked a bit further and I foundthe following difference:

My compiler inserts a reference to javax/servlet/ServletResponse class when the getOutpuStream is called. BJs compiler inserts a reference to javax/servlet/http/HttpServletResponse class for this method. Obviusly, btool can then not find the reference to javax/servlet because according to the class file there is no such reference.

Section 4.4.2 of the Java VM spec says:

class_index The value of the class_index item must be a valid index into the constant_pool table. The constant_pool entry at that index must be a CONSTANT_Class_info (§4.4.1) structure representing the class or interface type that contains the DECLARATION of the field or method.

I think the magic word is declaration ... this implies it must be the class that implements the method, which is a superclass, not the class that is used in the code.

So from my point of view it is a bug in BJ's compiler that might also affect other code :-( I am actually surprised that the VM does not complain.

bjhargrave commented 19 years ago

Comment author: Pavlin Dobrev <p.dobrev@prosyst.com>

In June I pointed that there is a problem in BTOOL if different version of JDK are used (at least with JDK 1.5) and I think the decision was to used JDK 1.4.2 as an official JVM for TCK and RI. Can we recompile the official RI and TCK build or we will mention this as a deviation?

bjhargrave commented 19 years ago

Comment author: @bjhargrave

No. I have my system configured to use Sun JDK 1.4.2 when using the ant scripts to compile.

 [echo] javac:                  c:/sunjdk14/bin/javac

C:\sunjdk14\bin>java -version java version "1.4.2_06" Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_06-b02) Java HotSpot(TM) Client VM (build 1.4.2_06-b02, mixed mode)

I just rebuilt (clean, publish) and the result still has the following import-package statement:

Import-Package: javax.crypto , javax.crypto.spec , javax.servlet.http , org.osgi.framework ;specification-version="1.3", org.osgi.service.http ;specification-version="1.2", org.osgi.test.cases.util ;specification-version="1.1", org.osgi.util.tracker ;specification-version="1.3"

bjhargrave commented 19 years ago

Comment author: @bjhargrave

Note: I did not do a complete clean build using ant from the command line. So some of the classes outside of the provisioning test project are probably compiled by Eclipse compiler and not Sun.

Peter, do you do a complete clean build from the command prompt? Or just run ant script from within Eclipse?

bjhargrave commented 19 years ago

Comment author: @bjhargrave

Pavlin, do you get this error if you run with the verifier off (-Xverify:none)?

bjhargrave commented 19 years ago

Comment author: Kevin Riff <kriff@espial.com>

Peter, you must be reading an old version of the spec. See http://java.sun.com/docs/books/vmspec/2nd-edition/ClassFileFormat-Java5.pdf instead. Although the URL says it's second edition for some reason, it's really 3rd edition. The changes from 2nd edition are highlighted with change bars. You'll see that the section you quoted was updated to read:

class_index The value of the class_index item must be a valid index into the constant_pool table. The constant_pool entry at that index must be a CONSTANT_Class_info (§4.5.1) structure representing a class or interface type that has the field or method as a MEMBER.

As I recall, this change was made in JDK 1.4 specifically to address an issue that affected OSGi. Imagine that instead of being public API, the javax.servlet package was a private implementation package that should never be accessed from outside the bundle. In that case, the bundle that contains it would not export it, but that bundle could still contain public classes in other packages that are exported (like javax.servlet.http). If your code calls a method that's really declared on a class in javax.servlet and your compiler follows the second edition rules then you get the situation that Pavlin reported. The classloader attempts to load a class in javax.servlet but fails because that class is not exported and therefore not accessible. But if the compiler instead inserts a reference to the class in javax.servlet.http then the classload will succeed and the usual method of resolving a method reference will locate the correct method in the class' superclass.

bjhargrave commented 19 years ago

Comment author: Pavlin Dobrev <p.dobrev@prosyst.com>

Pavlin, do you get this error if you run with the verifier off (- Xverify:none)?

Works. But if the verify is on ClassNotFound Exception apears

bjhargrave commented 19 years ago

Comment author: @bjhargrave

Works. But if the verify is on ClassNotFound Exception apears

OK. I think we are seeing this because of some strange behavior in the verifier.

Assume we have class a.A (package a) and b.B (package b). b.B extends a.A

Bundle X imports b but not a. If bundle X loads b.B, bundle X's class loader delegates the load request via the import to the bundle exporting b. Since b.B extends a.A, the class loader of b.B is asked to class load a.A (we assume that a.A is in the same bundle as b.B or that the bundle which export b imports a). This works because the VM asks b.B's class loader to find a.A.

However the verifier does not seem to follow these rules. When the verifier is verifying b.B and needs a.A it does not ask the class loader of b.B to locate a.A, it instead asks bundle X's class loader (the class loader who was originally asked to load b.B). But bundle X does not import a.A, so the verifier fails throwing a ClassNoDefFoundError. This is why turning the verifier off removes the problem.

Where are the details of the class lookup rules for the verifier? This seems an error that the verifier would use a different starting point to load a class that the VM.

Verification takes place during the linking phase [http://java.sun.com/docs/books/jls/third_edition/html/execution.html#12.1.2] which is done by ClassLoader.resolveClass [http://java.sun.com/j2se/1.4.2/docs/api/java/lang/ClassLoader.html#resolveClass(java.lang.Class)].

Typically, resolveClass is called by loadClass in the first class loader asked to load a class. In my example above this would be bundle X's class loader. What if we changes the class loader implementation to call resolveClass in the class loader which actually defined the class (i.e. in findClass rather than loadClass)? Perhaps the verifier is using the class loader which calls resolveClass to locate needed class rather than the class loader which loaded the class being verified? Making this change should allow the verifier to locate the necessary classes since the defining class loader must already have access to them.

Pavlin, I assume you are running this on the ProSyst framework implementation. Can you try an experiment and alter the class loader to call resolveClass in findClass (i.e. after defineClass is called) and see if the problem is resolved with the verifier running?

bjhargrave commented 19 years ago

Comment author: Kevin Riff <kriff@espial.com>

The process for verifying a class is spelled out in section 4.11 of the JVM specification. (See http://java.sun.com/docs/books/vmspec/2nd- edition/ClassFileFormat-Java5.pdf) I read through the section and didn't notice anything about which classloader should be used to load classes.

bjhargrave commented 19 years ago

Comment author: Kevin Riff <kriff@espial.com>

Here's an interesting bit from section 5.4.2 of the JVM spec where it discusses how classes are linked at runtime:

During preparation of a class or interface C, the Java virtual machine also

imposes loading constraints (§5.3.4). Let L1 be the defining loader of C. For each method m declared in C that overrides a method declared in a superclass or superinterface <D, L2 >, the Java virtual machine imposes the following loading constraints: Let T0 be the name of the type returned by m, and let T1, ..., Tn be the names of the argument types of m. Then Ti^L1 = Ti^L2 for i = 0 to n (§5.3.4).

If I'm reading this correctly, it's saying that if a class overrides a method from its superclass and they're loaded by different classloaders, then both classloaders MUST be able to access any classes used in the method's signature and they need to have access to the same class.

Peter, do you know if the BTool handles this sort of dependency properly? Classes used in a method (or field) signature won't necessarily be visible as CONSTANT_Class_info structures in the constant pool. You have to parse the descriptors in any CONSTANT_NameAndType_info entries as well to get the full dependency info. Does BTool do that?

bjhargrave commented 19 years ago

Comment author: Pavlin Dobrev <p.dobrev@prosyst.com>

According to the comments above I think that we must modify manifest of org.osgi.test.cases.provisioning and add javax.servlet in import clause.

It will be good also BTOOL to cover such scenarios.

For the r4 TCK passion initial provisioning will be deviation. What do you think?

bjhargrave commented 19 years ago

Comment author: @bjhargrave

According to the comments above I think that we must modify manifest of org.osgi.test.cases.provisioning and add javax.servlet in import clause.

Pavlin, did you try the experiment I suggested? I think this can be fixed by a change to the framework class loader implementation. I am not convinced this is a btool problem at this time.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

Svetozar will attach a small test case demonstrating the problem.

bjhargrave commented 18 years ago

Comment author: Svetozar Dimov <s_dimov@prosyst.bg>

Created attachment 2 resolving test

Attached file: ClassResolving.zip (application/x-zip-compressed, 6864 bytes) Description: resolving test

bjhargrave commented 18 years ago

Comment author: Svetozar Dimov <s_dimov@prosyst.bg>

The attached resolving test shows that the problematic class is defined & resolved, and the exception is thrown when the bundle attempts to instantiate it:

osgi> start 21 info: [HttpServiceController] - Unget on HttpService from bundle: 21 info: [HttpServiceController] - Building HttpService for bundle: 21 --- test class: class Test java.lang.NoClassDefFoundError: javax/servlet/Servlet at java.lang.Class.getDeclaredConstructors0(Native Method) at java.lang.Class.privateGetDeclaredConstructors(Class.java:1590) at java.lang.Class.getConstructor0(Class.java:1762) at java.lang.Class.newInstance0(Class.java:276) at java.lang.Class.newInstance(Class.java:259) at TestActivator.start(TestActivator.java:46)

With j9 v2.2 the result was: --- test class: class Test java.lang.NoClassDefFoundError: javax.servlet.Servlet at java.lang.Class.verifyImpl(Native Method) at java.lang.Class.verify(Class.java:253) at java.lang.Class.initialize(Class.java:315) at java.lang.Class.newInstanceImpl(Native Method) at java.lang.Class.newInstance(Class.java:1481) at TestActivator.start(TestActivator.java:46)

With some debug in the ProSyst framework, I got a bigger stack trace, just before the exception is thrown:

--- test class: class Test java.lang.Exception: --- at com.prosyst.mbs.impl.framework.DefaultClassProvider.loadClass(Default ClassProvider.java:226) at java.lang.ClassLoader.loadClass(Unknown Source) at java.lang.ClassLoader.loadClassInternal(Unknown Source) at java.lang.Class.getDeclaredConstructors0(Native Method) at java.lang.Class.privateGetDeclaredConstructors(Unknown Source) at java.lang.Class.getConstructor0(Unknown Source) at java.lang.Class.newInstance0(Unknown Source) at java.lang.Class.newInstance(Unknown Source) at TestActivator.start(TestActivator.java:46)

and there were no other calls to a bundle classloader before the --- test class: class Test dump & the exception; So I don't see a way to fix this in the bundle classloader.

With -noverify in the command line, no exceptions are thrown.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

I did some experimentation myself and confirmed your findings.

Test case overview

The test case has a BundleActivator class which uses ClassLoader.loadClass to load the Test class. It does this so the BundleActivator class does not have any class constants to the Test class. After getting the Class object for the Test class, newInstance is called which is when the problem surfaces. The Test class references an inner class that is a subclass of javax.servlet.http.HttpServlet. HttpServlet extends javax.servlet.GenericServlet which implements the javax.servlet.Servlet and javax.servlet.ServletConfig interfaces. The failure occurs because the verifier cannot load the javax.servlet.Servlet class during the execution of newInstance.

The test bundle is called ClassResolving and it only imports the javax.servlet.http package. It does not import the javax.servlet package since there are no references to the javax.servlet package in the class files. Normal class loading with the verifier off works fine because the ClassResolving bundle's classloader can load the javax.servlet.http.HttpServlet class by delegating to the servlet bundle because of the import package for the javax.servlet.http package. Then when the VM wants to load the javax.servlet.GenericServlet class, it asks the class loader of HttpServlet (ie. the servlet bundle's class loader) and not the ClassResolving bundle's class loader. This correctly loads the classes and things work fine.

However, when the verifier is not turned off, the verifier asks the ClassResolving bundle's class loader to load the javax.servlet.Servlet class and it cannot be found because the ClassResolving bundle does not import the javax.servlet package and has not direct access to load classes from that package. It seems that the verifier is improperly asking the Test (class being verified) classes classloader (ClassResolving bundle) to load the javax.servlet.Servlet class. Like normal class loading, the verifier should ask the referencing classes (javax.servlet.http.HttpServlet) classloader (servlet bundle) to load the class.

javax.servlet.Servlet appears to be the first class load attempt from the javax.servlet and javax.servlet.http packages. So even if resolveClass could solve the problem when called on javax.servlet.http.HttpServlet, no request to load HttpServlet has been made when the exception is thrown.

I have sent the test case onto IBM's JVM expert to enquire as to the proper behavior of the verifier since I would think that the verifier should follow normal class loading rules (i.e. the way it works for class loading when the verifier is off). I will report back when I know more.

bjhargrave commented 18 years ago

Comment author: @pkriens

I have updated Btool to parse all the CONSTANT_NameAndType info records in the constant pool. These point to a string with a descriptor of a method or field. This descriptor is parsed for its constituents and if these are classes, they are added to the set of classes to add.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

I don't think this is quite fixed. I rebuilt the org.osgi.test.cases.provisioning test case with the new btool and javax.servlet is listed twice :-)

   javax.servlet ;specification-version="2.1.0",
   javax.servlet;specification-version=2.1 ,

The latter statement is different in that it does not have a space after the package name and the spec version is not quoted or end with .0 like the others.

Here is the full generated import statement:

Import-Package: javax.crypto , javax.crypto.spec , javax.servlet ;specification-version="2.1.0", javax.servlet.http ;specification-version="2.1.0", javax.servlet;specification-version=2.1 , org.osgi.framework ;specification-version="1.3.0", org.osgi.service.http ;specification-version="1.2.0", org.osgi.test.cases.util ;specification-version="1.1.0", org.osgi.util.tracker ;specification-version="1.3.0"

bjhargrave commented 18 years ago

Comment author: @bjhargrave

Assign to Peter for btool fix.

bjhargrave commented 18 years ago

Comment author: @pkriens

The bug was closed, the provisioning project was not updated yet to remove the root property

bjhargrave commented 18 years ago

Comment author: @bjhargrave

Some post resolve notes on the problem. The load attempt for javax.servlet.Servlet by the verifier was not triggered by the anonymous inner class which extended javax.servlet.http.HttpService (implements Servlet) but was instead triggered by the call to HttpService.registerServlet which takes a Servlet as a formal parameter. So when the verifier was trying to verify the call site to the registerServlet method, it wanted to load the referenced Servlet type which was not directly accessible by the test code's class loader.

I was able to confirm this by commenting out the call to the registerServlet method which avoided the exception.

A CONSTANT_NameAndType entry contained the signature of the registerServlet method which contained the reference to the Servlet class. As described by Peter, a change was made to btool to process CONSTANT_NameAndType constant pool entries to add the necessry import of javax.servlet package to the manifest.