lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-7870) Use cl.loadClass(...) instead of Class.forName(..., cl) in SPIClassIterator
Date Fri, 09 Jun 2017 08:21:20 GMT

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

Uwe Schindler commented on LUCENE-7870:
---------------------------------------

bq. Is there a good place to lurk to watch the ”Lucene as Java 9 module” effort? An issue
to watch? A branch to track? If you are already making larger changes, it would be good to
have an eye on them from an OSGi perspective as well.

Not yet. There is no urgency about that. You may have seen the Java 9 developer panel at JAX
2017 (see videos). In general the Java 9 module system is fine for new developments, but porting
old applications to be modules is going to be hard, especially because of the split package
problem. This is unliekely to change fast in Lucene, sorry. My current recommendation is to
make the whole Lucene application a single module and go for it.

bq. I am not sure whether you are aware of buddy classloading, an OSGi extension in Equinox

This may help with the problem, but its still no standard way. OSGI is "broken" in that respect.
Java 9 has the default option and shares class loaders by default (which has pros and cons).
But I don't want to make this a Java 9 vs. OSGI discussion, I just mention this.

bq. Alas, buddy classloading won’t solve the present issue, as NamedSPILoader will currently
also use the thread context class loader (in addition to the class loader of lucene-core.jar
in steps 1.1 and 2.1). 

Of course. I made a proposal already to skip the stupid context classloader for Lucene's SPI
lookups and only allow to enable it for "broken" webapp context classloaders (unfortunately
this would only be possible via system property, as this must be setup very early during lucene-core
startup). Let's discuss this in another issue.

For your current problems: Even with module systems, I'd always prefer to Maven-Shade different
versions of the same lib- I'd also use the same approach like I'd suggest in Java 9: Package
everything needed from Lucene's classes into one OSGI bundle.

Finally, the Lucene does not offer OSGI bundles because of the split packages problem. Anybody
who want to use Lucene should setup a trivial Maven projects that depends on all required
Lucene JARs and after that uses Maven Shade plugin and the OSGI-BND plugin to create a encapsulated
singleton out of it. We know, that's not ideal, but running Lucene inside OSGI is a bad idea
anyways (huge performance issues). For desktop apps like Eclipse this may be fine, but not
for the main Lucene use case: Server side applications using Lucene running in a separate
JVM (Lucene does not coexist good with other stuff in the same JVM as it has very special
garbage collection requirements and uses heap and off-heap resources in very unconventional
ways). See Solr, Neo4J and Elasticsearch as typical examples.

> 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message