Petals ESB Container

Remove unneeded dependencies from the system classpath

Details

  • Type: Task Task
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 4.2.4
  • Fix Version/s: 5.0.0
  • Component/s: Classloader
  • Security Level: Public
  • Description:
    Hide

    The system classloader (the one with the dependencies in the classpath argument of the java commandline starting Petals) contains the jar of the launcher.
    This jar refers to a lot of dependencies and thus populates the system classloader (used in particular by all deployed JBI components) with them.

    It would be better to reduce as much as possible the dependencies on the system classloader and instead load the extra dependencies from the Petals launcher classloader inside the launcher.

    Show
    The system classloader (the one with the dependencies in the classpath argument of the java commandline starting Petals) contains the jar of the launcher. This jar refers to a lot of dependencies and thus populates the system classloader (used in particular by all deployed JBI components) with them. It would be better to reduce as much as possible the dependencies on the system classloader and instead load the extra dependencies from the Petals launcher classloader inside the launcher.
  • Environment:
    -

Issue Links

Activity

Victor NOËL made changes - Mon, 23 Mar 2015 - 16:39:49 +0100
Field Original Value New Value
Assignee Christophe DENEUX [ cdeneux ] Victor NOËL [ vnoel ]
Hide
Victor NOËL added a comment - Tue, 24 Mar 2015 - 14:49:52 +0100

It would maybe make sense, instead of tweaking the launcher classloader, to tweak the components classloader so that its parent is not the system classloader but its parent (the extension classloader, or even maybe the bootstrap classloader).

This would allow to remove complex classloader code from petals launcher and to be sure the component classloader never get unwanted dependencies.

Show
Victor NOËL added a comment - Tue, 24 Mar 2015 - 14:49:52 +0100 It would maybe make sense, instead of tweaking the launcher classloader, to tweak the components classloader so that its parent is not the system classloader but its parent (the extension classloader, or even maybe the bootstrap classloader). This would allow to remove complex classloader code from petals launcher and to be sure the component classloader never get unwanted dependencies.
Hide
Victor NOËL added a comment - Thu, 26 Mar 2015 - 16:59:30 +0100

The previous comment is not correct (because the container needs to share the JBI classloader with the components).

A proposed solution is as follow:

  • The launcher has its classloader as before, no assumption is made on it, it would be best for it to be reduced as it will still hold some dependencies in memory that won't be reused by the container, maybe it should even not include the container dependencies since they will be loaded directly from the container's classloader.
  • When it launches petals, the container sets up (as a hierarchy):
  • the JBI classloader containing petals-jbi
  • the platform classloader containing petals-commons-log (and/or other things??)
  • its classloader
  • When a component is created, its classloader is created from:
  • the platform classloader if the server properties have isolated as true
  • the container classloader if not (but is there any reason to do that??)

Finally, some care must be taken so that there is not security/correctness/performance leaks with statics in the JBI and platform classloader that would let components share some state/influence each other/lock each others. (I'm thinking for example of the jbi descriptor builder, but there can be other things maybe…).

Show
Victor NOËL added a comment - Thu, 26 Mar 2015 - 16:59:30 +0100 The previous comment is not correct (because the container needs to share the JBI classloader with the components). A proposed solution is as follow:
  • The launcher has its classloader as before, no assumption is made on it, it would be best for it to be reduced as it will still hold some dependencies in memory that won't be reused by the container, maybe it should even not include the container dependencies since they will be loaded directly from the container's classloader.
  • When it launches petals, the container sets up (as a hierarchy):
  • the JBI classloader containing petals-jbi
  • the platform classloader containing petals-commons-log (and/or other things??)
  • its classloader
  • When a component is created, its classloader is created from:
  • the platform classloader if the server properties have isolated as true
  • the container classloader if not (but is there any reason to do that??)
Finally, some care must be taken so that there is not security/correctness/performance leaks with statics in the JBI and platform classloader that would let components share some state/influence each other/lock each others. (I'm thinking for example of the jbi descriptor builder, but there can be other things maybe…).
Victor NOËL made changes - Thu, 26 Mar 2015 - 16:59:43 +0100
Status New [ 10000 ] Open [ 10002 ]
Priority Major [ 3 ]
Hide
Victor NOËL added a comment - Tue, 5 May 2015 - 16:00:29 +0200

Apparently needed dependencies in the system classloader are:

  • petals-log:
    • java commons logging insist on using the system classloader to find them… (see also PETALSDISTRIB-135)
  • petals-common-log:
    • components, container and petals-log use the execution context and its static thread local
    • they also uses the LogData class (this could be taken care of because in the end it's just a map, so we could use that to replace it)
  • Most easycommons lib
    • they are dependencies of the previous dependencies
  • and petals-jbi obviously
Show
Victor NOËL added a comment - Tue, 5 May 2015 - 16:00:29 +0200 Apparently needed dependencies in the system classloader are:
  • petals-log:
    • java commons logging insist on using the system classloader to find them… (see also PETALSDISTRIB-135)
  • petals-common-log:
    • components, container and petals-log use the execution context and its static thread local
    • they also uses the LogData class (this could be taken care of because in the end it's just a map, so we could use that to replace it)
  • Most easycommons lib
    • they are dependencies of the previous dependencies
  • and petals-jbi obviously
Victor NOËL made changes - Wed, 6 May 2015 - 11:50:52 +0200
Status Open [ 10002 ] In Progress [ 10003 ]
Hide
Victor NOËL added a comment - Mon, 18 May 2015 - 16:20:36 +0200 - edited

slf4j-api and slf4j-jdk14 must also be added to the system classloader, see PETALSDISTRIB-139 for details. (decision reverted, see next comments)

Show
Victor NOËL added a comment - Mon, 18 May 2015 - 16:20:36 +0200 - edited slf4j-api and slf4j-jdk14 must also be added to the system classloader, see PETALSDISTRIB-139 for details. (decision reverted, see next comments)
Hide
Christophe DENEUX added a comment - Mon, 18 May 2015 - 16:28:15 +0200

I don't understand why you must add slf4j-api and slf4j-jdk14 at system classloader level. In my mind, it is sufficient to add it in the JBI components that need them to be able to have log traces of embedded libraries

Show
Christophe DENEUX added a comment - Mon, 18 May 2015 - 16:28:15 +0200 I don't understand why you must add slf4j-api and slf4j-jdk14 at system classloader level. In my mind, it is sufficient to add it in the JBI components that need them to be able to have log traces of embedded libraries
Hide
Victor NOËL added a comment - Mon, 18 May 2015 - 16:33:15 +0200

I would like to do that for several reasons:

  • first it will save space
  • second it is used also by the container so, we might as well share it too
  • third, it makes sense that we "select" the slf4j implementation at the last moment, i.e. in the system classloader and the jdk14 implementation depends on slf4j-api…

But we can decide to do differently, what is your opinion in the light of these reasons?

Show
Victor NOËL added a comment - Mon, 18 May 2015 - 16:33:15 +0200 I would like to do that for several reasons:
  • first it will save space
  • second it is used also by the container so, we might as well share it too
  • third, it makes sense that we "select" the slf4j implementation at the last moment, i.e. in the system classloader and the jdk14 implementation depends on slf4j-api…
But we can decide to do differently, what is your opinion in the light of these reasons?
Hide
Victor NOËL added a comment - Mon, 18 May 2015 - 17:20:45 +0200

Ok, actually I think it's better not to include them to limit mandatory version of dependencies.

Show
Victor NOËL added a comment - Mon, 18 May 2015 - 17:20:45 +0200 Ok, actually I think it's better not to include them to limit mandatory version of dependencies.
Hide
Christophe DENEUX added a comment - Tue, 19 May 2015 - 08:42:15 +0200

To avoid library version conflicts at component level we must have a minimum number of libraries in the system classloader. When a class was loaded by the system classloader all inherithed classloaders can't load another version of the class even if provided by the component classloader. So, if a component requires another version of slf4j-api or slf4j-jdk14, that will not work. In the past, we have already have this library version conflict problem.

The space saved is not a problem. Petals ESB does not consume a lot of memory.

In the system class loader, just put librairies required by petals bootstrap. If a container library requires another library that is not in the system classloader, put it in the container class loader, even if this library can be used by components. Same for JBI components.

Show
Christophe DENEUX added a comment - Tue, 19 May 2015 - 08:42:15 +0200 To avoid library version conflicts at component level we must have a minimum number of libraries in the system classloader. When a class was loaded by the system classloader all inherithed classloaders can't load another version of the class even if provided by the component classloader. So, if a component requires another version of slf4j-api or slf4j-jdk14, that will not work. In the past, we have already have this library version conflict problem. The space saved is not a problem. Petals ESB does not consume a lot of memory. In the system class loader, just put librairies required by petals bootstrap. If a container library requires another library that is not in the system classloader, put it in the container class loader, even if this library can be used by components. Same for JBI components.
Hide
Victor NOËL added a comment - Thu, 28 May 2015 - 14:36:26 +0200

geronimo-jta was added to the system classpath: it is needed if we want to be able to get the TransactionManager from a component that does not share its classloader with the container, see PETALSESBCONT-2 for details.

It's a stable enough API and meant to be part of Java EE so I think it's ok to have it there.

Show
Victor NOËL added a comment - Thu, 28 May 2015 - 14:36:26 +0200 geronimo-jta was added to the system classpath: it is needed if we want to be able to get the TransactionManager from a component that does not share its classloader with the container, see PETALSESBCONT-2 for details. It's a stable enough API and meant to be part of Java EE so I think it's ok to have it there.
Christophe DENEUX made changes - Thu, 28 May 2015 - 15:51:13 +0200
Link This issue blocks PETALSESBCONT-2 [ PETALSESBCONT-2 ]
Hide
Victor NOËL added a comment - Fri, 3 Jul 2015 - 13:31:14 +0200

New problem: petals-log depends on too much things such as easycommons-utils which share pools of transformers for the whole VM, including components.
Even though it saves space, it's not very safe to share all of that, and it creates problem with dependencies to the XSLT transformer used: the component can't use their own transformer (such as xalan or an older version of Saxon).

We should reduce the dependencies used by petals-log as much as possible then, see PETALSDISTRIB-147.

Show
Victor NOËL added a comment - Fri, 3 Jul 2015 - 13:31:14 +0200 New problem: petals-log depends on too much things such as easycommons-utils which share pools of transformers for the whole VM, including components. Even though it saves space, it's not very safe to share all of that, and it creates problem with dependencies to the XSLT transformer used: the component can't use their own transformer (such as xalan or an older version of Saxon). We should reduce the dependencies used by petals-log as much as possible then, see PETALSDISTRIB-147.
Victor NOËL made changes - Fri, 3 Jul 2015 - 13:31:24 +0200
Link This issue depends on PETALSDISTRIB-147 [ PETALSDISTRIB-147 ]
Hide
Victor NOËL added a comment - Mon, 6 Jul 2015 - 16:44:50 +0200

easycommons-util and Saxon-HE were removed from the system classpath (see PETALSDISTRIB-147).

This means for example that the Transformers pool provided by easycommons-util is not available to all classloaders anymore: there will be one per component and one for the kernel (not that PetalsPayloadDumperFileHandler for the logs uses the pool of the kernel container, see PETALSDISTRIB-147 for details).

Show
Victor NOËL added a comment - Mon, 6 Jul 2015 - 16:44:50 +0200 easycommons-util and Saxon-HE were removed from the system classpath (see PETALSDISTRIB-147). This means for example that the Transformers pool provided by easycommons-util is not available to all classloaders anymore: there will be one per component and one for the kernel (not that PetalsPayloadDumperFileHandler for the logs uses the pool of the kernel container, see PETALSDISTRIB-147 for details).
Victor NOËL made changes - Wed, 8 Jul 2015 - 11:33:44 +0200
Link This issue depends on PETALSDISTRIB-135 [ PETALSDISTRIB-135 ]
Victor NOËL made changes - Wed, 8 Jul 2015 - 14:16:16 +0200
Link This issue blocks PETALSESBCONT-255 [ PETALSESBCONT-255 ]
Hide
Victor NOËL added a comment - Tue, 21 Jul 2015 - 17:21:36 +0200

petals-commons (after being cleaned, now it only includes BytesSource) was added to the system classpath (see PETALSESBCONT-327).

Show
Victor NOËL added a comment - Tue, 21 Jul 2015 - 17:21:36 +0200 petals-commons (after being cleaned, now it only includes BytesSource) was added to the system classpath (see PETALSESBCONT-327).
Victor NOËL made changes - Tue, 21 Jul 2015 - 17:41:46 +0200
Link This issue depends on PETALSESBCONT-327 [ PETALSESBCONT-327 ]
Hide
Victor NOËL added a comment - Tue, 22 Sep 2015 - 09:34:00 +0200

This can be marked as fixed. Further changes to the system classpath can have their own issues in the future.

Show
Victor NOËL added a comment - Tue, 22 Sep 2015 - 09:34:00 +0200 This can be marked as fixed. Further changes to the system classpath can have their own issues in the future.
Victor NOËL made changes - Tue, 22 Sep 2015 - 09:34:00 +0200
Status In Progress [ 10003 ] Resolved [ 10004 ]
Fix Version/s 5.0.0 [ 10413 ]
Resolution Fixed [ 1 ]
Transition Status Change Time Execution Times Last Executer Last Execution Date
New New Open Open
3d 20m
1
Victor NOËL
Thu, 26 Mar 2015 - 16:59:43 +0100
Open Open In Progress In Progress
40d 17h 51m
1
Victor NOËL
Wed, 6 May 2015 - 11:50:52 +0200
In Progress In Progress Resolved Resolved
138d 21h 43m
1
Victor NOËL
Tue, 22 Sep 2015 - 09:34:00 +0200



People

Dates

  • Created:
    Mon, 23 Mar 2015 - 16:39:43 +0100
    Updated:
    Tue, 22 Sep 2015 - 09:34:00 +0200
    Resolved:
    Tue, 22 Sep 2015 - 09:33:59 +0200