Skip to content

[FELIX-6274] Fix possible LogService ClassCastException #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tjwatson merged 2 commits into from
Jun 16, 2020
Merged

[FELIX-6274] Fix possible LogService ClassCastException #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f44235
[FELIX-6274] Add a test demonstrating the LogService ClassCastException
timothyjward May 13, 2020
a9be2a9
[FELIX-6274] - Stop supporting pre R7 LogService for the Logger
tjwatson May 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions scr/bnd.bnd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ Export-Package: org.apache.felix.scr.component;version=1.1.0;provide:=true, \
Private-Package: org.apache.felix.scr.impl.*

# Configuration Admin is optional and dynamic, but allow eager wiring by importing it
# LogService is optional but if present the R4.0 version 1.3 is sufficient.
# LogService is optional but if present the R7.0 version 1.4 is sufficient.
# Metatype import is optional and dynamic, but allow eager wiring by importing it
# PackageAdmin is used to find reference types if the component's bundle does not import it.
# R4.0 version 1.2 is sufficient.
# optional import for Gogo annotations
Import-Package: \
org.osgi.service.cm;version="[1.6,2)";resolution:=optional, \
org.osgi.service.log;version="[1.3,2)";resolution:=optional, \
org.osgi.service.log;version="[1.4,2)";resolution:=optional, \
org.osgi.service.metatype;version="[1.2,2)";resolution:=optional, \
org.osgi.service.packageadmin;version="[1.2,2)";resolution:=optional,\
org.apache.felix.service.command;resolution:=optional, \
*

DynamicImport-Package: \
org.osgi.service.cm;version="[1.6,2)", \
org.osgi.service.log;version="[1.3,2)", \
org.osgi.service.log;version="[1.4,2)", \
org.osgi.service.metatype;version="[1.1,2)"
8 changes: 4 additions & 4 deletions scr/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.service.log</artifactId>
<version>1.3.0</version>
<version>1.4.0</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -149,21 +149,21 @@
<dependency>
<groupId>org.ops4j.pax.exam</groupId>
<artifactId>pax-exam-container-forked</artifactId>
<version>3.2.0</version>
<version>4.13.3</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.ops4j.pax.exam</groupId>
<artifactId>pax-exam-junit4</artifactId>
<version>3.2.0</version>
<version>4.13.3</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.ops4j.pax.exam</groupId>
<artifactId>pax-exam-link-mvn</artifactId>
<version>3.2.0</version>
<version>4.13.3</version>
<scope>test</scope>
</dependency>

Expand Down
69 changes: 37 additions & 32 deletions scr/src/main/java/org/apache/felix/scr/impl/Activator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

import org.apache.felix.scr.impl.config.ScrConfigurationImpl;
import org.apache.felix.scr.impl.inject.internal.ClassUtils;
import org.apache.felix.scr.impl.logger.InternalLogger.Level;
import org.apache.felix.scr.impl.logger.ScrLogger;
import org.apache.felix.scr.impl.manager.ComponentHolder;
import org.apache.felix.scr.impl.metadata.ComponentMetadata;
Expand All @@ -59,7 +60,6 @@
import org.osgi.namespace.extender.ExtenderNamespace;
import org.osgi.service.component.ComponentConstants;
import org.osgi.service.component.runtime.ServiceComponentRuntime;
import org.osgi.service.log.LogService;

/**
* This activator is used to cover requirement described in section 112.8.1 @@ -27,14
Expand Down Expand Up @@ -137,7 +137,9 @@ public void restart(boolean globalExtender)
}
if ( ClassUtils.m_packageAdmin != null )
{
logger.log(LogService.LOG_INFO, "Stopping to restart with new globalExtender setting: {0}", null, globalExtender);
logger.log(Level.INFO,
"Stopping to restart with new globalExtender setting: {0}", null,
globalExtender);

//this really is a restart, not the initial start
// the initial start where m_globalContext is null should skip this as m_packageAdmin should not yet be set.
Expand All @@ -150,21 +152,20 @@ public void restart(boolean globalExtender)
// logger might be null
if ( logger != null )
{
logger.log(LogService.LOG_ERROR, "Exception stopping during restart", e);
logger.log(Level.ERROR, "Exception stopping during restart", e);
}
}
// reinstantiate logger
logger = new ScrLogger(m_configuration, m_context);
}
try
{
logger.log(LogService.LOG_INFO, "Starting with globalExtender setting: {0}", null, globalExtender);
logger.log(Level.INFO, "Starting with globalExtender setting: {0}", null,
globalExtender);

super.start( m_globalContext );
}
catch ( final Exception e )
{
logger.log(LogService.LOG_ERROR, "Exception starting during restart", e);
logger.log(Level.ERROR, "Exception starting during restart", e);
}

}
Expand All @@ -184,7 +185,7 @@ protected void doStart() throws Exception
m_componentRegistry.setRegistration(m_runtime_reg);

// log SCR startup
logger.log( LogService.LOG_INFO, " Version = {0}",
logger.log(Level.INFO, " Version = {0}",
null, m_bundle.getVersion().toString() );

// create and start the component actor
Expand All @@ -207,6 +208,7 @@ public void stop(BundleContext context) throws Exception
super.stop( context );
m_configuration.stop();
store(m_componentMetadataStore, context, logger, m_configuration.cacheMetadata());
logger.closeTracker();
}

@Override
Expand Down Expand Up @@ -277,7 +279,7 @@ private static ConcurrentMap<Long, List<ComponentMetadata>> load(
}
catch (IOException e)
{
logger.log(LogService.LOG_WARNING,
logger.log(Level.WARN,
"Error loading component metadata cache.", e);
}
}
Expand All @@ -286,7 +288,7 @@ private static ConcurrentMap<Long, List<ComponentMetadata>> load(
catch (RuntimeException re)
{
// avoid failing all of SCR start on cache load bug
logger.log(LogService.LOG_ERROR,
logger.log(Level.ERROR,
"Error loading component metadata cache.", re);
return new ConcurrentHashMap<>();
}
Expand Down Expand Up @@ -339,7 +341,7 @@ private static void store(Map<Long, List<ComponentMetadata>> componentsMap,
}
catch (IOException e)
{
logger.log(LogService.LOG_WARNING, "Error storing component metadata cache.",
logger.log(Level.WARN, "Error storing component metadata cache.",
e);
}
}
Expand Down Expand Up @@ -376,13 +378,6 @@ public void doStop() throws Exception
m_componentActor.terminate();
m_componentActor = null;
}

// close the LogService tracker now
if ( logger != null )
{
logger.close();
logger = null;
}
ClassUtils.close();
}

Expand Down Expand Up @@ -418,7 +413,9 @@ public void start()
catch ( final InterruptedException e )
{
Thread.currentThread().interrupt();
logger.log(LogService.LOG_WARNING, "The wait for {0} being destroyed before destruction has been interrupted.", e,
logger.log(Level.WARN,
"The wait for {0} being destroyed before destruction has been interrupted.",
e,
bundle );
}
loadComponents( ScrExtension.this.bundle );
Expand All @@ -445,7 +442,9 @@ public void destroy()
catch ( final InterruptedException e )
{
Thread.currentThread().interrupt();
logger.log(LogService.LOG_WARNING, "The wait for {0} being started before destruction has been interrupted.", e,
logger.log(Level.WARN,
"The wait for {0} being started before destruction has been interrupted.",
e,
bundle );

}
Expand Down Expand Up @@ -494,7 +493,7 @@ private void loadComponents(Bundle bundle)
BundleContext context = bundle.getBundleContext();
if ( context == null )
{
logger.log(LogService.LOG_DEBUG, "Cannot get BundleContext of {0}.", null, bundle);
logger.log(Level.DEBUG, "Cannot get BundleContext of {0}.", null, bundle);

return;
}
Expand All @@ -509,7 +508,8 @@ private void loadComponents(Bundle bundle)
{
if ( !m_bundle.adapt( BundleRevision.class ).equals( wire.getProvider() ) )
{
logger.log(LogService.LOG_DEBUG, "{0} wired to a different extender: {1}.", null,
logger.log(Level.DEBUG, "{0} wired to a different extender: {1}.",
null,
bundle, wire.getProvider().getBundle());

return;
Expand Down Expand Up @@ -540,7 +540,8 @@ private void loadComponents(Bundle bundle)
// terminate if already loaded (or currently being loaded)
if ( loaded )
{
logger.log(LogService.LOG_DEBUG, "Components for {0} already loaded. Nothing to do.", null,
logger.log(Level.DEBUG,
"Components for {0} already loaded. Nothing to do.", null,
bundle );

return;
Expand Down Expand Up @@ -578,12 +579,15 @@ private void loadComponents(Bundle bundle)

if ( e instanceof IllegalStateException && bundle.getState() != Bundle.ACTIVE )
{
logger.log(LogService.LOG_DEBUG, "{0} has been stopped while trying to activate its components. Trying again when the bundles gets started again.", e,
logger.log(Level.DEBUG,
"{0} has been stopped while trying to activate its components. Trying again when the bundles gets started again.",
e,
bundle );
}
else
{
logger.log(LogService.LOG_ERROR, "Error while loading components of {0}", e, bundle);
logger.log(Level.ERROR, "Error while loading components of {0}", e,
bundle);
}
}
}
Expand All @@ -610,39 +614,40 @@ private void disposeComponents(Bundle bundle)
}
catch ( Exception e )
{
logger.log(LogService.LOG_ERROR, "Error while disposing components of {0}", e, bundle);
logger.log(Level.ERROR, "Error while disposing components of {0}", e,
bundle);
}
}
}

@Override
protected void debug(final Bundle bundle, final String msg)
{
if ( logger.isLogEnabled(LogService.LOG_DEBUG) )
if (logger.isLogEnabled(Level.DEBUG))
{
if ( bundle != null )
{
logger.log( LogService.LOG_DEBUG, "{0} : " + msg, null, bundle );
logger.log(Level.DEBUG, "{0} : " + msg, null, bundle);
}
else
{
logger.log( LogService.LOG_DEBUG, msg, null );
logger.log(Level.DEBUG, msg, null);
}
}
}

@Override
protected void warn(final Bundle bundle, final String msg, final Throwable t)
{
if ( logger.isLogEnabled(LogService.LOG_WARNING) )
if (logger.isLogEnabled(Level.WARN))
{
if ( bundle != null )
{
logger.log( LogService.LOG_WARNING, "{0} : " + msg, t, bundle );
logger.log(Level.WARN, "{0} : " + msg, t, bundle);
}
else
{
logger.log( LogService.LOG_WARNING, msg, t );
logger.log(Level.WARN, msg, t);
}
}
}
Expand Down
Loading