Issue Details (XML | Word | Printable)

Key: GLASSFISH-15545
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Shing Wai Chan
Reporter: Bruno Borges
Votes: 1
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
glassfish

TLD files located in JARs of a shared library are not mapped

Created: 12/Jan/11 07:39 AM   Updated: 18/Jan/11 05:57 PM  Due: 18/Jan/11   Resolved: 14/Jan/11 04:24 PM
Component/s: web_container
Affects Version/s: v3.0.1
Fix Version/s: None

Time Tracking:
Not Specified

File Attachments: 1. File glassfish-notworking.ear (4 kB) 13/Jan/11 01:52 PM - Bruno Borges
2. File glassfish-working.ear (2.62 MB) 13/Jan/11 01:54 PM - Bruno Borges

Environment:

Linux


Tags: 3_1-approved
Participants: arungupta, Bruno Borges, Chris Kasso, Chris Perry, kchung, Sanjeeb Sahoo and Shing Wai Chan


 Description  « Hide

When frameworks such Struts that provide TLD descriptor files are located in a shared library, applications that use them fail to process the tag libraries.



Sanjeeb Sahoo added a comment - 12/Jan/11 08:31 AM

-> jsp


arungupta added a comment - 12/Jan/11 08:51 AM

Here is what Kinman mentioned in a private thread to Bruno ...

In GlassFish3, system jars that contain TLDs need to implement TldProvider interface somewhere. The purpose of such
instances is to to tell the JSP engine that TLDs are present in the jars and need to be scanned. This is needed for performance and to cope with the new classloader hierarchy. We have such implementations for JSTL and JSF, but not for struts, since it is not bundled with GF.


Shing Wai Chan added a comment - 13/Jan/11 11:38 AM

Can you attach a simple test war?


Shing Wai Chan added a comment - 13/Jan/11 11:44 AM - edited

How bad is its impact? (Severity)
medium. Workaround: put the jars in WEB-INF/lib

How often does it happen? Will many users see this problem? (Frequency)
common library outside the application with tld

How much effort is required to fix it? (Cost)
low. Add a new TldProvider to scan jars in lib and domains/domain1/lib

What is the risk of fixing it and how will the risk be mitigated? (Risk)
low


Bruno Borges added a comment - 13/Jan/11 01:52 PM - edited

To test this issue, install Struts2 essential libraries to GlassFish's shared lib location.

Then deploy 'glassfish-nonworking.ear'

Access http://localhost:8080/tldissue/index.jsp


Bruno Borges added a comment - 13/Jan/11 01:54 PM - edited

To test the application running fine, make sure there are no struts2 libraries at GF's SL.

Deploy 'glassfish-working.ear'

Access http://localhost:8080/tldissue/index.jsp


Chris Perry added a comment - 13/Jan/11 02:07 PM

So we have to rebuild every single application to deploy our third party jars inside each one (to improve performance no less)? One of the nice things with having system jars is removing the risk of differing versions of shared libraries from getting deployed.

Regardless, in trying to follow the work around for this bug I can't for the life of me figure out how TldProvider is supposed to be implemented. There is marginally more than 0 documentation for it. I have added an implementation to my system jar, but none of the methods in it are ever called. (And I feel funky about adding an import for org.glassfish.api.web.TldProvider, feels very non-portable).

Am I just stupid here or what? Is there an example of an external taglib library that implements this anywhere that can be used as a reference?


Shing Wai Chan added a comment - 13/Jan/11 02:17 PM

Since there are several jars in $INSTALL_ROOT/lib, scanning it in startup will increase the startup time.
So, we plan to scan $DOMAIN_ROOT/lib only at this moment.

In this case, any custom libraries jar can be put there and the tld will be scanned. I have verified that Bruno's test case is working if one put all those jar under $DOMAIN_ROOT/lib (eg. domains/domain1/lib).


Chris Perry added a comment - 13/Jan/11 02:48 PM

You say "we plan to scan $DOMAIN_ROOT/lib only at this moment". Does that mean that 3.0.1 b22 is supposed to work that way right now, or do you mean in some newer version?

Right now 3.0.1 b22 does not work (for me at least) if the jar is placed in $DOMAIN_ROOT/lib or any of its subdirectories. If I put the jar in WEB-INF/lib then it does.

I tried this with Bruno's test case, moving all of the libs from the webapp's META-INF/lib to $DOMAIN_ROOT/lib and it does not work for me: org.apache.jasper.JasperException: /index.jsp(1,42) PWC6117: File "/struts-tags" not found


Bruno Borges added a comment - 13/Jan/11 02:57 PM

Chan, I ran my tests on top of GF 3.0.1 b22.

It does not work.

And when I say "shared library" I mean domains/domain1/lib.

I also tried putting Struts2 JAR files in domains/domain1/lib/applibs.

Didn't work either.


Shing Wai Chan added a comment - 13/Jan/11 03:01 PM - edited

I am still waiting for approval to checkin the code.


Bruno Borges added a comment - 13/Jan/11 03:02 PM

What if the JAR scan process of shared library was parallel to the startup process?

And while it does not finish and "parent first" apps do not find something, a thread would wait.

Useless feature for production environment, but a must at development time.


Chris Perry added a comment - 13/Jan/11 04:40 PM

Since this setup already seems to stray from the spec a little, how about a properties file or something to point to shared libs that have embedded tld's. I would have to think that would be much faster than this contract arrangement, and a whole lot simpler. Plus, it would give you the ability to identify tlds in 3rd party libs that you can't modify (it isn't clear to me whether the TldProvider mechanism makes this possible or not since I couldn't get it to work). Just my $.02


Shing Wai Chan added a comment - 13/Jan/11 07:07 PM

We plan to scan the customize jars as follows:
1. those in domains/domain1/lib
2. those specified in --libraries during the deployment time


Sanjeeb Sahoo added a comment - 13/Jan/11 07:45 PM

Isn't it too late to make any change in GlassFish 3.1 to support this requirement which seems like an RFE to me. Does the Servlet/JSP spec say whether shared jars need to be scanned for TLDs?


Shing Wai Chan added a comment - 13/Jan/11 07:47 PM

Even though the glassfish-working.ear run, I see the following in server.log:
Caused by: java.lang.ClassNotFoundException: javassist.ClassPool
at org.glassfish.web.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1518)
at org.glassfish.web.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1368)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:169)
at ognl.OgnlRuntime.<clinit>(OgnlRuntime.java:162)
... 63 more

I have searched through the ear files, there is no such class.
It would be good to update the applications so that we can verify it end to end here without problem.


Chris Perry added a comment - 14/Jan/11 02:44 AM

Sanjeeb appears to be correct in that the spec makes no requirement to scan shared jars as I read it. I had always presumed that it did. So this is really just a loss of a non-standard feature from v2 to v3. I guess I can deal with that as long as the resultant implementation complies with the spec.


Chris Perry added a comment - 14/Jan/11 03:05 AM

As a follow up for anyone else running into this: What we did to make this work was move our tlds out of our shared library into a separate jar. That separate jar gets packaged with our app, but the shared library still goes in $DOMAIN_ROOT/lib. This configuration works, and actually makes sense to me. You then only have to deal with updating deployed applications on changes to the shared library if one of the tags that they use gets modified in a manner that requires a change to the tld. And in this case you would likely have had to update your deployed app anyway.


Bruno Borges added a comment - 14/Jan/11 04:16 AM

This issue could also be implemented as a configurable option (either enabling/disabling TLD scan on shared libraries; disabled by default).

Corporate deployments usually throw common JARs of corporate frameworks to shared lib to reduce size of EARs and facilitate delivery and deployment of projects by having a small EAR file without JARs.

One of our customers have 300+ apps deployed on WebSphere servers, and they are used to do this.

If GlassFish wants to be an option for such use case, this issue is a must have.


Sanjeeb Sahoo added a comment - 14/Jan/11 09:08 AM

Although I am in favor of improving tld packaging to encourage modularity, I must say that this request has come in way too late for 3.1 release and that's my primary objection. We should look into this in next release. Currently we are in high resistance mode [1]. Again, this is just my opinion, not the opinion of the people who actually approve a change in high resistance mode.

[1] http://wikis.sun.com/display/GlassFish/3.1ChangeControl#3.1ChangeControl-ChangeControlProcessforHighResistanceMode


Shing Wai Chan added a comment - 14/Jan/11 10:45 AM

I find the issue in the test jar. javassist-3.7.ga.jar is missing. After adding the jar, the exception is resolved.


Bruno Borges added a comment - 14/Jan/11 11:23 AM

This is what I think about this issue, considering the Change Control Process:

1. How bad?

  • Is a regression of functionality or performance available in a prior release
  • Likely to generate a customer support call
  • An in-your-face issue that will touch the majority of users

2. How often?
Always

3. How much?
Low

4. What risk?
Low

5. Workaround exist?
Yes

6. Describe in release note?
Yes


Chris Kasso added a comment - 14/Jan/11 03:03 PM

Approved for 3.1 limiting the change to only scanning <domain>/lib and --libraries.


Shing Wai Chan added a comment - 14/Jan/11 04:24 PM

Sending src/main/java/com/sun/enterprise/web/WebModule.java
Sending src/main/java/com/sun/enterprise/web/WebModuleListener.java
Adding src/main/java/org/glassfish/web/LibrariesTldProvider.java
Transmitting file data ...
Committed revision 44519.


Sanjeeb Sahoo added a comment - 17/Jan/11 08:20 AM

Looking at the changes, I think it is incomplete as it does not seem to be processing Class-Path manifest entries of library jars for potential source of TLDs. More over, it does not exclude jars from library directory that that are excluded from server runtime.


Shing Wai Chan added a comment - 17/Jan/11 10:35 PM

Per discussion with Sahoo, since we don't process the glassfish/lib, then there is no need to process Exclude marker.


kchung added a comment - 18/Jan/11 10:46 AM

Scanning jars referenced in Manifest Class-Path is currently limited to only the set of referencing jars (i.e. those with Manifest Class_path) that are in the application (i.e. under WEB-INF). See TldScanner for further details. This is a design decision to limit the amount scanning at server startup time. This should not be an issue in practice, as it is extremely unlikely to happen for system jars. The user can always make sure to put all the necessary jar in domain1/lib.


Bruno Borges added a comment - 18/Jan/11 10:50 AM

Now I understood Sahoos' previous comment.

Yes, there's no need to scan for referenced jars on MANIFEST Class-Path, since most certainly they are side-by-side on domain1/lib


Sanjeeb Sahoo added a comment - 18/Jan/11 05:57 PM

IIUC, the fix made by Shing Wai also covers per application libraries (domain/lib/applib). Now, it is often recommended to use Class-Path in application libraries to make sure classpath order is maintained. If you search glassfish forum, you will find examples of such usage.