[jira] [Created] (CHAIN-62) Use of thread context ClassLoader under OSGi

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Created] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
Use of thread context ClassLoader under OSGi
--------------------------------------------

                 Key: CHAIN-62
                 URL: https://issues.apache.org/jira/browse/CHAIN-62
             Project: Commons Chain
          Issue Type: Bug
    Affects Versions: 1.2
         Environment: OSGi
            Reporter: Ales Dolecek
            Priority: Minor


The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.

This leads to problems with ConfigCatalogRule and especislly LookupCommand.

Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.

I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either

a) detect that it is used inside OSGi framework, or
b) provide static boolean flag to disable it

Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.

Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13182397#comment-13182397 ]

Simone Tripodi commented on CHAIN-62:
-------------------------------------

Good catch Ales,

what about if we provide 2 methods:

 * {{CatalogFactory#getInstance(ClassLoader)}}
 * {{CatalogFactory#getInstance()}} which uses {{CatalogFactory.class.getClassLoader}}?

would it be helpful to be used inside an OSGi container?
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183294#comment-13183294 ]

Ales Dolecek commented on CHAIN-62:
-----------------------------------

Hello,

  I'm afraid it won't help. I can actually "simulate" this with following code:

ClassLoader savedCl = Thread.currentThread().getContextClassLoader();
try {
  Thread.currentThread().getContextClassLoader(getClass().getClassLoader());
  // do something
} finally {
  Thread.currentThread().getContextClassLoader(savedCl);
}

I choose getClass().getClassLoader() which is actually the bundle class loader (provided that the class that contain the code is in my bundle).

There are two problems:

a) initial population of Catalog with commands
  - suppose two different bundles each populating one catalog
  - the bundles will use different classloaders
  - not problem with Digester that is creating the commands
    - if both bundles import same packages their *different* classloaders will see *same* classes
  - commons chains will however assign the catalog to different catalog factory

  => The bundles must agree on class loader to use
    - best bet is CatalogFactory.class.getClassLoader()
    - but that class loader would not see the classes of commands provided by the bundles (because digester has also to use thread context class loader as explained above)

  => The bundles therefore must use classloader of class from bundle which imports *all* required classes that implement some commands added to catalogs. Clearly against the modularity concept of OSGi.

  It is "catch 22" situation - either satisfy CatalogFactory and Digester will be "blind" or give Digester "usefull" class loader and you are forced to temporary set it every time you use the chains.

b) lookup command
  - the lookup command uses CatalogFactory instance to find the target command
  - if it is not provided then CatalogFactory#getInstance() is used
  - since it is the Digester who actually create/configure the instance you can't control which CatalogFactory will the lookup instance use
  => you need to add rule that assigns some specific CatalogFactory to every LookupCommand
  => this means tweaking the rules used by digester which might be very problematic
    - although you can generally check that the class specified by the command creation rule is (subclass of) LookupCommand it does not work 100% correct since there might be classes that actually delegate to LookupCommand like:

  class MyCommand implements Filter {

    private LookupCommand delegate;

  boolean execute(Context context) {
    return delegate.execute(context);
  }

  boolean postprocess(Context context, Exception exception) {
    return delegate.postprocess(context, exception);
  }
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Updated] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

     [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ales Dolecek updated CHAIN-62:
------------------------------

    Comment: was deleted

(was: What about following patch?

Index: src/main/java/org/apache/commons/chain/CatalogFactory.java
===================================================================
--- src/main/java/org/apache/commons/chain/CatalogFactory.java  (revision 1229598)
+++ src/main/java/org/apache/commons/chain/CatalogFactory.java  (working copy)
@@ -49,6 +49,11 @@
     public static final String DELIMITER = ":";


+    /**
+     * Default catalog factory to use.
+     */
+    public static CatalogFactory INSTANCE;
+
     // --------------------------------------------------------- Public Methods


@@ -178,17 +183,20 @@


     /**
-     * <p>Return the singleton {@link CatalogFactory} instance
-     * for the relevant <code>ClassLoader</code>.  For applications
-     * that use a thread context class loader (such as web applications
-     * running inside a servet container), this will return a separate
-     * instance for each application, even if this class is loaded from
+     * <p>Returns the {@link #INSTANCE} is set. Otherwise returns singleton
+     * {@link CatalogFactory} instance for the relevant <code>ClassLoader</code>.
+     * For applications that use a thread context class loader (such as web
+     * applications running inside a servet container), this will return a
+     * separate instance for each application, even if this class is loaded from
      * a shared parent class loader.</p>
      *
      * @return the per-application singleton instance of {@link CatalogFactory}
      */
     public static CatalogFactory getInstance() {

+        if (INSTANCE != null)
+            return INSTANCE;
+
         CatalogFactory factory = null;
         ClassLoader cl = getClassLoader();
         synchronized (factories) {)
   

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183335#comment-13183335 ]

Ales Dolecek commented on CHAIN-62:
-----------------------------------

What about following patch?

Index: src/main/java/org/apache/commons/chain/CatalogFactory.java
===================================================================
--- src/main/java/org/apache/commons/chain/CatalogFactory.java  (revision 1229598)
+++ src/main/java/org/apache/commons/chain/CatalogFactory.java  (working copy)
@@ -49,6 +49,11 @@
     public static final String DELIMITER = ":";


+    /**
+     * Default catalog factory to use.
+     */
+    public static CatalogFactory INSTANCE;
+
     // --------------------------------------------------------- Public Methods


@@ -178,17 +183,20 @@


     /**
-     * <p>Return the singleton {@link CatalogFactory} instance
-     * for the relevant <code>ClassLoader</code>.  For applications
-     * that use a thread context class loader (such as web applications
-     * running inside a servet container), this will return a separate
-     * instance for each application, even if this class is loaded from
+     * <p>Returns the {@link #INSTANCE} is set. Otherwise returns singleton
+     * {@link CatalogFactory} instance for the relevant <code>ClassLoader</code>.
+     * For applications that use a thread context class loader (such as web
+     * applications running inside a servet container), this will return a
+     * separate instance for each application, even if this class is loaded from
      * a shared parent class loader.</p>
      *
      * @return the per-application singleton instance of {@link CatalogFactory}
      */
     public static CatalogFactory getInstance() {

+        if (INSTANCE != null)
+            return INSTANCE;
+
         CatalogFactory factory = null;
         ClassLoader cl = getClassLoader();
         synchronized (factories) {
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Updated] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

     [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ales Dolecek updated CHAIN-62:
------------------------------

    Attachment: CatalogFactory.patch

What about following patch?
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Issue Comment Edited] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183294#comment-13183294 ]

Ales Dolecek edited comment on CHAIN-62 at 1/10/12 3:44 PM:
------------------------------------------------------------

Hello,

  I'm afraid it won't help. I can actually "simulate" this with following code:

ClassLoader savedCl = Thread.currentThread().getContextClassLoader();
try {
  Thread.currentThread().getContextClassLoader(getClass().getClassLoader());
  // do something
} finally {
  Thread.currentThread().getContextClassLoader(savedCl);
}

I choose getClass().getClassLoader() which is actually the bundle class loader (provided that the class that contain the code is in my bundle).

There are two problems and neither calls CatalogFactory#getInstance() directly so adding overloaded version does not help.

a) initial population of Catalog with commands
  - suppose two different bundles each populating one catalog
  - the bundles will use different classloaders
  - not problem with Digester that is creating the commands
    - if both bundles import same packages their *different* classloaders will see *same* classes
  - commons chains will however assign the catalog to different catalog factory

  => The bundles must agree on class loader to use
    - best bet is CatalogFactory.class.getClassLoader()
    - but that class loader would not see the classes of commands provided by the bundles (because digester has also to use thread context class loader as explained above)

  => The bundles therefore must use classloader of class from bundle which imports *all* required classes that implement some commands added to catalogs. Clearly against the modularity concept of OSGi.

  It is "catch 22" situation - either satisfy CatalogFactory and Digester will be "blind" or give Digester "usefull" class loader and you are forced to temporary set it every time you use the chains.

b) lookup command
  - the lookup command uses CatalogFactory instance to find the target command
  - if it is not provided then CatalogFactory#getInstance() is used
  - since it is the Digester who actually create/configure the instance you can't control which CatalogFactory will the lookup instance use
  => you need to add rule that assigns some specific CatalogFactory to every LookupCommand
  => this means tweaking the rules used by digester which might be very problematic
    - although you can generally check that the class specified by the command creation rule is (subclass of) LookupCommand it does not work 100% correct since there might be classes that actually delegate to LookupCommand like:

  class MyCommand implements Filter {

    private LookupCommand delegate;

  boolean execute(Context context) {
    return delegate.execute(context);
  }

  boolean postprocess(Context context, Exception exception) {
    return delegate.postprocess(context, exception);
  }
               
      was (Author: alesd):
    Hello,

  I'm afraid it won't help. I can actually "simulate" this with following code:

ClassLoader savedCl = Thread.currentThread().getContextClassLoader();
try {
  Thread.currentThread().getContextClassLoader(getClass().getClassLoader());
  // do something
} finally {
  Thread.currentThread().getContextClassLoader(savedCl);
}

I choose getClass().getClassLoader() which is actually the bundle class loader (provided that the class that contain the code is in my bundle).

There are two problems:

a) initial population of Catalog with commands
  - suppose two different bundles each populating one catalog
  - the bundles will use different classloaders
  - not problem with Digester that is creating the commands
    - if both bundles import same packages their *different* classloaders will see *same* classes
  - commons chains will however assign the catalog to different catalog factory

  => The bundles must agree on class loader to use
    - best bet is CatalogFactory.class.getClassLoader()
    - but that class loader would not see the classes of commands provided by the bundles (because digester has also to use thread context class loader as explained above)

  => The bundles therefore must use classloader of class from bundle which imports *all* required classes that implement some commands added to catalogs. Clearly against the modularity concept of OSGi.

  It is "catch 22" situation - either satisfy CatalogFactory and Digester will be "blind" or give Digester "usefull" class loader and you are forced to temporary set it every time you use the chains.

b) lookup command
  - the lookup command uses CatalogFactory instance to find the target command
  - if it is not provided then CatalogFactory#getInstance() is used
  - since it is the Digester who actually create/configure the instance you can't control which CatalogFactory will the lookup instance use
  => you need to add rule that assigns some specific CatalogFactory to every LookupCommand
  => this means tweaking the rules used by digester which might be very problematic
    - although you can generally check that the class specified by the command creation rule is (subclass of) LookupCommand it does not work 100% correct since there might be classes that actually delegate to LookupCommand like:

  class MyCommand implements Filter {

    private LookupCommand delegate;

  boolean execute(Context context) {
    return delegate.execute(context);
  }

  boolean postprocess(Context context, Exception exception) {
    return delegate.postprocess(context, exception);
  }
                 

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with ConfigCatalogRule and especislly LookupCommand.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Updated] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

     [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Simone Tripodi updated CHAIN-62:
--------------------------------

    Description:
The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.

This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.

Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.

I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either

a) detect that it is used inside OSGi framework, or
b) provide static boolean flag to disable it

Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.

Ales

  was:
The CatalogFactory#getInstance() is using thread context ClassLoader which gives undefined behavior under OSGi.

This leads to problems with ConfigCatalogRule and especislly LookupCommand.

Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.

I think that CatalogFactory#getClassLoader() might allow disabling use of thread context class loader - either

a) detect that it is used inside OSGi framework, or
b) provide static boolean flag to disable it

Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.

Ales

   

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183348#comment-13183348 ]

Simone Tripodi commented on CHAIN-62:
-------------------------------------

Great to see you contributing Ales! may I can ask you to provide please a patch that includes a testcase that proves the correctness inside OSGi?

As a side note: code in the comment is not very human-friendly, do you mind of taking care of formatting? You can use wiki-alike tags, press on the {{?}} button help... TIA!
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Issue Comment Edited] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183294#comment-13183294 ]

Ales Dolecek edited comment on CHAIN-62 at 1/10/12 8:56 PM:
------------------------------------------------------------

Hello,

  I'm afraid it won't help. I can actually "simulate" this with following code:

{code}
ClassLoader savedCl = Thread.currentThread().getContextClassLoader();
try {
  Thread.currentThread().getContextClassLoader(getClass().getClassLoader());
  // do something
} finally {
  Thread.currentThread().getContextClassLoader(savedCl);
}
{code}

I choose {{getClass().getClassLoader()}} which is actually the bundle class loader (provided that the class that contain the code is in my bundle).

There are two problems and neither calls {{CatalogFactory#getInstance()}} directly so adding overloaded version does not help.

a) initial population of Catalog with commands
  - suppose two different bundles each populating one catalog
  - the bundles will use different classloaders
  - not problem with Digester that is creating the commands
    - if both bundles import same packages their *different* classloaders will see *same* classes
  - commons chains will however assign the catalog to different catalog factory

  => The bundles must agree on class loader to use
    - best bet is {{CatalogFactory.class.getClassLoader()}}
    - but that class loader would not see the classes of commands provided by the bundles (because digester has also to use thread context class loader as explained above)

  => The bundles therefore must use classloader of class from bundle which imports *all* required classes that implement some commands added to catalogs. Clearly against the modularity concept of OSGi.

  It is "catch 22" situation - either satisfy CatalogFactory and Digester will be "blind" or give Digester "usefull" class loader and you are forced to temporary set it every time you use the chains.

b) lookup command
  - the lookup command uses CatalogFactory instance to find the target command
  - if it is not provided then {{CatalogFactory#getInstance()}} is used
  - since it is the Digester who actually create/configure the instance you can't control which CatalogFactory will the lookup instance use
  => you need to add rule that assigns some specific CatalogFactory to every LookupCommand
  => this means tweaking the rules used by digester which might be very problematic
    - although you can generally check that the class specified by the command creation rule is (subclass of) LookupCommand it does not work 100% correct since there might be classes that actually delegate to LookupCommand like:

{code}
  class MyCommand implements Filter {

    private LookupCommand delegate;

  boolean execute(Context context) {
    return delegate.execute(context);
  }

  boolean postprocess(Context context, Exception exception) {
    return delegate.postprocess(context, exception);
  }
{code}

               
      was (Author: alesd):
    Hello,

  I'm afraid it won't help. I can actually "simulate" this with following code:

ClassLoader savedCl = Thread.currentThread().getContextClassLoader();
try {
  Thread.currentThread().getContextClassLoader(getClass().getClassLoader());
  // do something
} finally {
  Thread.currentThread().getContextClassLoader(savedCl);
}

I choose getClass().getClassLoader() which is actually the bundle class loader (provided that the class that contain the code is in my bundle).

There are two problems and neither calls CatalogFactory#getInstance() directly so adding overloaded version does not help.

a) initial population of Catalog with commands
  - suppose two different bundles each populating one catalog
  - the bundles will use different classloaders
  - not problem with Digester that is creating the commands
    - if both bundles import same packages their *different* classloaders will see *same* classes
  - commons chains will however assign the catalog to different catalog factory

  => The bundles must agree on class loader to use
    - best bet is CatalogFactory.class.getClassLoader()
    - but that class loader would not see the classes of commands provided by the bundles (because digester has also to use thread context class loader as explained above)

  => The bundles therefore must use classloader of class from bundle which imports *all* required classes that implement some commands added to catalogs. Clearly against the modularity concept of OSGi.

  It is "catch 22" situation - either satisfy CatalogFactory and Digester will be "blind" or give Digester "usefull" class loader and you are forced to temporary set it every time you use the chains.

b) lookup command
  - the lookup command uses CatalogFactory instance to find the target command
  - if it is not provided then CatalogFactory#getInstance() is used
  - since it is the Digester who actually create/configure the instance you can't control which CatalogFactory will the lookup instance use
  => you need to add rule that assigns some specific CatalogFactory to every LookupCommand
  => this means tweaking the rules used by digester which might be very problematic
    - although you can generally check that the class specified by the command creation rule is (subclass of) LookupCommand it does not work 100% correct since there might be classes that actually delegate to LookupCommand like:

  class MyCommand implements Filter {

    private LookupCommand delegate;

  boolean execute(Context context) {
    return delegate.execute(context);
  }

  boolean postprocess(Context context, Exception exception) {
    return delegate.postprocess(context, exception);
  }
                 

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183572#comment-13183572 ]

Ales Dolecek commented on CHAIN-62:
-----------------------------------

Hello,

  I haven't wrote any unit tests for OSGi as I do not know how can I effectively run them. And this case would be rather problematic as:

a) it requires 2 different bundles to exploit the wrong behavior, and
b) is OSGi might be OSGi framework dependent

I can, however provide simple test case that calls {{CatalogFactory#getInstance()}} with different thread context class loaders and confirm that

a) the method returns different CatalogFactory for each call if the INSTANCE is null, and
b) the method returns same CatalogFactory if the INSTANCE is set and that the factory is the INSTANCE

Is this sufficient?

Ales
               

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Issue Comment Edited] (CHAIN-62) Use of thread context ClassLoader under OSGi

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/CHAIN-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13183572#comment-13183572 ]

Ales Dolecek edited comment on CHAIN-62 at 1/10/12 9:19 PM:
------------------------------------------------------------

Hello,

  I haven't wrote any unit tests for OSGi as I do not know how can I effectively run them. And this case would be rather problematic as:

a) it requires 2 different bundles to exploit the wrong behavior, and
b) as OSGi it might be OSGi framework dependent (I use equinox)

I can, however provide simple test case that calls {{CatalogFactory#getInstance()}} with different thread context class loaders and confirm that

a) the method returns different CatalogFactory for each call if the INSTANCE is null, and
b) the method returns same CatalogFactory if the INSTANCE is set and that the factory is the INSTANCE

Is this sufficient?

And BTW: Is it OK to have public non-final variable CatalogFactory or should I provide static setter?

Ales
               
      was (Author: alesd):
    Hello,

  I haven't wrote any unit tests for OSGi as I do not know how can I effectively run them. And this case would be rather problematic as:

a) it requires 2 different bundles to exploit the wrong behavior, and
b) is OSGi might be OSGi framework dependent

I can, however provide simple test case that calls {{CatalogFactory#getInstance()}} with different thread context class loaders and confirm that

a) the method returns different CatalogFactory for each call if the INSTANCE is null, and
b) the method returns same CatalogFactory if the INSTANCE is set and that the factory is the INSTANCE

Is this sufficient?

Ales
                 

> Use of thread context ClassLoader under OSGi
> --------------------------------------------
>
>                 Key: CHAIN-62
>                 URL: https://issues.apache.org/jira/browse/CHAIN-62
>             Project: Commons Chain
>          Issue Type: Bug
>    Affects Versions: 1.2
>         Environment: OSGi
>            Reporter: Ales Dolecek
>            Priority: Minor
>         Attachments: CatalogFactory.patch
>
>
> The {{CatalogFactory#getInstance()}} is using thread context ClassLoader which gives undefined behavior under OSGi.
> This leads to problems with {{ConfigCatalogRule}} and especially {{LookupCommand}}.
> Two bundles wired to same commons chain may use different catalog factories when parsing commands from XML or looking up commands from catalog.
> I think that {{CatalogFactory#getClassLoader()}} might allow disabling use of thread context class loader - either
> a) detect that it is used inside OSGi framework, or
> b) provide static boolean flag to disable it
> Combination of both might be via use of bundle activator that would set the flag. The activator would be used only under OSGi acting as "auto-detection" and still some other bundle might revert to default if required.
> Ales

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira