[
https://issues.apache.org/jira/browse/LUCENE-7870?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16043000#comment-16043000
]
Uwe Schindler edited comment on LUCENE-7870 at 6/8/17 4:44 PM:
---------------------------------------------------------------
Hi,
bq. I do see, however, that java.util.ServiceLoader (which Lucene tries to
mimic/replace) uses Class.forName(...), so that may be a sign.
That is exactly the reason why it is made like this. In general we would likely
to get rid of the Context ClassLoader in the SPI lookups of Lucene (you see we
fall back to using the JAR's classloader, for looking up). IMHO, we should
never touch the context loader, but as Java's service loader does this, too, we
kept it for compatibility.
Nevertheless, this is a problem of Eclipse/OSGI, because if Lucene would use
the "original" ServiceLoader, it would cause the same problems for you. And it
that case you would not even be able to "patch" Lucene. So fix the problem
correctly! (see below) I know SPI and OSGI don't like each other but looking at
Java 9's module system, they have a perfect way to handle this. With later
Lucene versions we will switch to Java 9's ServiceLoader, so you can declare
the services in the module declaration.
I'd separate the context class loader for those 2 apps (and set it equal to the
loader that loaded the bundle).
was (Author: thetaphi):
Hi,
bq. I do see, however, that java.util.ServiceLoader (which Lucene tries to
mimic/replace) uses Class.forName(...), so that may be a sign.
That is exactly the reason why it is made like this. In general we would likely
to get rid of the Context ClassLoader in the SPI lookups of Lucene (you see we
fall back to using the JAR's classloader, for looking up). IMHO, we should
never touch the context loader, but as Java's service loader does this, too, we
kept it for compatibility.
Nevertheless, this is a problem of Eclipse/OSGI, because if Lucene would use
the "original" ServiceLoader, it would cause the same problems for you. And it
that case you would not even be able to "patch" Lucene. So fix the problem
correctly. I know SPI and OSGI don't like each other but looking at Java 9's
module system, they have a perfect way to handle this. With later Lucene
versions we will switch to Java 9's ServiceLoader, so you can declare the
services in the module declaration.
I'd separate the context class loader for those 2 apps (and set it equal to the
loader that loaded the bundle).
> Use cl.loadClass(...) instead of Class.forName(..., cl) in SPIClassIterator
> ---------------------------------------------------------------------------
>
> Key: LUCENE-7870
> URL: https://issues.apache.org/jira/browse/LUCENE-7870
> Project: Lucene - Core
> Issue Type: Bug
> Affects Versions: 5.2.1, 6.1
> Environment: Eclipse Equinox 4.7
> Reporter: Andreas Sewe
>
> This issue is initially described in [Eclipse Bug
> 517935|https://bugs.eclipse.org/bugs/show_bug.cgi?id=517935] and prevents
> multiple versions of Lucene Core coexisting in an Equinox environment (FYI,
> Equinox is the OSGi container used by the Eclipse IDE).
> Here’s how the problem manifests: While Equinox cleanly separates two
> versions of the same Lucene Core bundle (e.g.,
> {{org.apache.lucene.core_5.2.1}} and {{org.apache.lucene.core_6.1.0}}) using
> two different bundle class loaders, it uses a single context classloader for
> all threads: the
> [{{ContextFinder}}|https://wiki.eclipse.org/Context_Class_Loader_Enhancements#Context_Finder_2].
> When asked to load a class, the {{ContextFinder}} *initiates* a search
> through all bundle class loaders currently “on“ the call stack; the class to
> be loaded is then *defined* by the respective bundle’s class loader.
> And here is where the use of {{Class.forName(..., classLoader)}} rather than
> {{classLoader.loadClass(...)}} causes problems. Consider the following
> sequence of events:
> # The {{NamedSPILoader}} of bundle {{o.a.l.core_5.2.1}} (re)loads some
> service (e.g., the {{Lucene50PostingFormat}}).
> ## It (through {{SPIClassIterator}}) first uses {{Class.forName}} with the
> bundle class loader of {{o.a.l.core_5.2.1}} (as per LUCENE-4713) to
> successfully load {{Lucene50PostingFormat}} from the {{o.a.l.core_5.2.1}}
> bundle.
> ## Then (through another {{SPIClassIterator}}) it uses {{Class.forName}} with
> the thread’s context class loader (here: {{ContextFinder}}) to load
> {{Lucene50PostingFormat}}. The {{ContextFinder}} delegates loading to the
> {{o.a.l.core_5.2.1}} bundle’s class loader, as that bundle is topmost on the
> call stack. This bundle class loader (again) successfully loads
> {{Lucene50PostingFormat}} from the {{o.a.l.core_5.2.1}} bundle.
> # Later, the {{NamedSPILoader}} of *another* bundle {{o.a.l.core_6.1.0}}
> loads the {{Lucene50PostingFormat}} service.
> ## It (through {{SPIClassIterator}}) first uses {{Class.forName}} with the
> bundle class loader of {{o.a.l.core_6.1.0}} to successfully load
> {{Lucene50PostingFormat}} from the {{o.a.l.core_6.1.0}} bundle.
> ## Then (through another {{SPIClassIterator}}) it uses {{Class.forName}} with
> the thread’s context class loader (the same {{ContextFinder}} again) to load
> {{Lucene50PostingFormat}}. As {{Class.forName}} remembers that the
> {{ContextFinder}} has successfully initiated the loading of
> {{Lucene50PostingFormat}} before, it simply returns the {{Class}} instance
> defined earlier in step _1.2_. But that class was defined by a different
> bundle class loader, namely that of {{o.a.l.core_5.2.1}}! This cache look up
> happens in native code; the {{ContextFinder}}‘s {{loadClass}} method isn’t
> even called, so there’s no way it can load the class from the
> {{o.a.l.core_6.1.0}} bundle, even though it now is topmost on the stack.
> ## Casting the {{Lucene50PostingFormat}} loading from bundle
> {{o.a.l.core_5.2.1}} to {{PostingFormat}} from bundle {{o.a.l.core_6.1.0}}
> then fails, leaving the {{o.a.l.core_6.1.0}} bundle in a broken state.
> It {{SPIClassIterator.next()}} would use {{classLoader.loadClass(...)}}
> rather than {{Class.forName(..., classLoader}}), then class loading in step
> _1.2_ wouldn’t *lock in* the {{Lucene50PostingFormat}} class from
> {{org.apache.lucene.core_5.2.1}}; instead, step _2.2_ would perform a
> completely independent look up that retrieves the class from the correct
> bundle. The cast in step _2.3_ would then succeed.
> At Eclipse Orbit, we plan to distribute a [patched
> version|https://git.eclipse.org/r/98804/] of Lucene Core, but obviously we
> would like to see the (one-line) change included in the upstream project.
> BTW, if you fear that bypassing {{Class.forName}} “caching” affects
> performance, then there’s no need to worry: Most {{ClassLoader}}
> implementations cache as well ({{findLoadedClass}}); it’s only
> {{ContextFinder}} that [overrides {{loadClass}}
> wholesale|https://git.eclipse.org/c/equinox/rt.equinox.framework.git/tree/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/ContextFinder.java?h=R4_7_maintenance],
> as it dynamically (based on the current call stack) delegates to the
> (caching) bundle class loaders.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
