You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Ceki Gülcü <ce...@qos.ch> on 2005/01/12 13:26:57 UTC
Removing PluginClassloader
Hello Paul, Scott,
Here is a patch that gets rid of PluginClassLoaderFactory class in
o.a.l.chainsaw.plugins.
To accomodate this change it renames
LogUI.loadConfigurationUsingPluginClassLoader() as
logUI.loadConfiguration(URL). It also removes the dependency on
PluginClassLoaderFactory in o.a.l.c.receivers.ReceiversHelper.
Would it be OK with you if it was applied?
Here is the patch.
Remove:
src/java/org/apache/log4j/chainsaw/plugins/PluginClassLoaderFactory.java
Index: src/java/org/apache/log4j/chainsaw/LogUI.java
===================================================================
RCS file:
/home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/LogUI.java,v
retrieving revision 1.116
diff -u -r1.116 LogUI.java
--- src/java/org/apache/log4j/chainsaw/LogUI.java 2 Jan 2005 09:51:40
-0000 1.116
+++ src/java/org/apache/log4j/chainsaw/LogUI.java 12 Jan 2005
12:18:45 -0000
@@ -93,7 +93,6 @@
import org.apache.log4j.chainsaw.icons.ChainsawIcons;
import org.apache.log4j.chainsaw.icons.LineIconFactory;
import org.apache.log4j.chainsaw.messages.MessageCenter;
-import org.apache.log4j.chainsaw.plugins.PluginClassLoaderFactory;
import org.apache.log4j.chainsaw.prefs.LoadSettingsEvent;
import org.apache.log4j.chainsaw.prefs.SaveSettingsEvent;
import org.apache.log4j.chainsaw.prefs.SettingsListener;
@@ -311,7 +310,7 @@
config = config.trim();
try {
URL configURL = new URL(config);
- logUI.loadConfigurationUsingPluginClassLoader(configURL);
+ logUI.loadConfiguration(configURL);
}catch(MalformedURLException e) {
logger.error("Failed to convert config string to url", e);
}
@@ -1315,7 +1314,7 @@
new Thread(
new Runnable() {
public void run() {
- loadConfigurationUsingPluginClassLoader(url);
+ loadConfiguration(url);
receiversPanel.updateReceiverTreeInDispatchThread();
}
@@ -1834,20 +1833,11 @@
*
* @param url
*/
- private void loadConfigurationUsingPluginClassLoader(final URL url) {
- ClassLoader classLoader =
PluginClassLoaderFactory.getInstance().getClassLoader();
- ClassLoader previousTCCL =
Thread.currentThread().getContextClassLoader();
+ private void loadConfiguration(final URL url) {
if(url!=null) {
- try {
- // we temporarily swap the TCCL so that plugins can find
resources
- Thread.currentThread().setContextClassLoader(classLoader);
- JoranConfigurator jc = new JoranConfigurator();
- jc.doConfigure(url, LogManager.getLoggerRepository());
- }finally{
- // now switch it back...
- Thread.currentThread().setContextClassLoader(previousTCCL);
- }
+ JoranConfigurator jc = new JoranConfigurator();
+ jc.doConfigure(url, LogManager.getLoggerRepository());
}
ensureChainsawAppenderHandlerAdded();
}
Index: src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java
===================================================================
RCS file:
/home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java,v
retrieving revision 1.9
diff -u -r1.9 ReceiversHelper.java
--- src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java 24
Nov 2004 08:17:02 -0000 1.9
+++ src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java 12
Jan 2005 12:18:45 -0000
@@ -26,7 +26,7 @@
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
-import org.apache.log4j.chainsaw.plugins.PluginClassLoaderFactory;
+import org.apache.log4j.helpers.Loader;
/**
@@ -57,23 +57,18 @@
stream = new LineNumberReader(new
InputStreamReader(url.openStream()));
String line;
- // we need the special Classloader, because under Web start,
optional jars might be local
- // to this workstation
- ClassLoader classLoader =
PluginClassLoaderFactory.getInstance().getClassLoader();
-
while ((line = stream.readLine()) != null) {
try {
if (line.startsWith("#") || (line.length() == 0)) {
continue;
}
- Class receiverClass = classLoader.loadClass(line);
+ Class receiverClass = Loader.loadClass(line);
receiverClassList.add(receiverClass);
logger.debug("Located known Receiver class " +
receiverClass.getName());
} catch (ClassNotFoundException e) {
- logger.warn("Failed to locate Receiver class:" + line);
- }
- catch (NoClassDefFoundError e) {
+ logger.warn("Failed to locate Receiver class: {}",
line);
+ } catch (NoClassDefFoundError e) {
logger.error("Failed to locate Receiver class:" +
line + ", looks like a dependent class is missing from the classpath", e);
}
}
--
Ceki Gülcü
The complete log4j manual: http://www.qos.ch/log4j/
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Removing PluginClassloader
Posted by Paul Smith <ps...@aconex.com>.
First I apologise if my email came across harsh (it does to me now that
I read it again) I blame it on the jetlag and the cold weather. :D
(it's 10 degrees C in London at the moment, and 37 degrees back home in
Melbourne)
Could I ask for a bit longer to think about this before committing
either way? I agree that if there is no use for it (and currently
DB/JMS Receiver's must be loaded from the ext/lib ) it should be dropped
before the final release.
cheers,
Paul Smith
Ceki Gülcü wrote:
>
> Recalling a previous discussion [1], I was under the (possibly wrong?)
> impression that you were also convinced that dropping the
> PluginRepositoryFactory would be good thing. Setting the thread
> context class loader, unless absolutely necessary, usually amounts to
> bad practice. Anyway, if you feel that it works in this particular
> case, I'll happily drop my suggestion and let you decide.
>
> [1] http://marc.theaimsgroup.com/?l=log4j-dev&m=110288749417366&w=2
>
> At 03:18 PM 1/12/2005, Paul Smith wrote:
>
>> Ceki,
>>
>> I know you have a problem with classloader trickery, and I'm sure
>> it's for good reason, but I'm not sure why we have to remove it for
>> Chainsaw? Is it really that big a deal? Chainsaw is for all intents
>> and purposes an application built around log4j, rather than a core
>> log4j component. Why would having a custom Classloader in Chainsaw
>> cause any problems?
>>
>> Am I just a bit naive about it all?
>>
>> cheers,
>>
>> Paul Smith
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Removing PluginClassloader
Posted by Ceki Gülcü <ce...@qos.ch>.
Recalling a previous discussion [1], I was under the (possibly wrong?)
impression that you were also convinced that dropping the
PluginRepositoryFactory would be good thing. Setting the thread context
class loader, unless absolutely necessary, usually amounts to bad
practice. Anyway, if you feel that it works in this particular case, I'll
happily drop my suggestion and let you decide.
[1] http://marc.theaimsgroup.com/?l=log4j-dev&m=110288749417366&w=2
At 03:18 PM 1/12/2005, Paul Smith wrote:
>Ceki,
>
>I know you have a problem with classloader trickery, and I'm sure it's for
>good reason, but I'm not sure why we have to remove it for Chainsaw? Is
>it really that big a deal? Chainsaw is for all intents and purposes an
>application built around log4j, rather than a core log4j component. Why
>would having a custom Classloader in Chainsaw cause any problems?
>
>Am I just a bit naive about it all?
>
>cheers,
>
>Paul Smith
--
Ceki Gülcü
The complete log4j manual: http://www.qos.ch/log4j/
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Removing PluginClassloader
Posted by Paul Smith <ps...@aconex.com>.
Ceki,
I know you have a problem with classloader trickery, and I'm sure it's
for good reason, but I'm not sure why we have to remove it for
Chainsaw? Is it really that big a deal? Chainsaw is for all intents
and purposes an application built around log4j, rather than a core log4j
component. Why would having a custom Classloader in Chainsaw cause any
problems?
Am I just a bit naive about it all?
cheers,
Paul Smith
Ceki Gülcü wrote:
>
> Hello Paul, Scott,
>
> Here is a patch that gets rid of PluginClassLoaderFactory class in
> o.a.l.chainsaw.plugins.
> To accomodate this change it renames
> LogUI.loadConfigurationUsingPluginClassLoader() as
> logUI.loadConfiguration(URL). It also removes the dependency on
> PluginClassLoaderFactory in o.a.l.c.receivers.ReceiversHelper.
>
> Would it be OK with you if it was applied?
>
> Here is the patch.
>
> Remove:
> src/java/org/apache/log4j/chainsaw/plugins/PluginClassLoaderFactory.java
>
> Index: src/java/org/apache/log4j/chainsaw/LogUI.java
> ===================================================================
> RCS file:
> /home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/LogUI.java,v
> retrieving revision 1.116
> diff -u -r1.116 LogUI.java
> --- src/java/org/apache/log4j/chainsaw/LogUI.java 2 Jan 2005
> 09:51:40 -0000 1.116
> +++ src/java/org/apache/log4j/chainsaw/LogUI.java 12 Jan 2005
> 12:18:45 -0000
> @@ -93,7 +93,6 @@
> import org.apache.log4j.chainsaw.icons.ChainsawIcons;
> import org.apache.log4j.chainsaw.icons.LineIconFactory;
> import org.apache.log4j.chainsaw.messages.MessageCenter;
> -import org.apache.log4j.chainsaw.plugins.PluginClassLoaderFactory;
> import org.apache.log4j.chainsaw.prefs.LoadSettingsEvent;
> import org.apache.log4j.chainsaw.prefs.SaveSettingsEvent;
> import org.apache.log4j.chainsaw.prefs.SettingsListener;
> @@ -311,7 +310,7 @@
> config = config.trim();
> try {
> URL configURL = new URL(config);
> - logUI.loadConfigurationUsingPluginClassLoader(configURL);
> + logUI.loadConfiguration(configURL);
> }catch(MalformedURLException e) {
> logger.error("Failed to convert config string to url", e);
> }
> @@ -1315,7 +1314,7 @@
> new Thread(
> new Runnable() {
> public void run() {
> - loadConfigurationUsingPluginClassLoader(url);
> + loadConfiguration(url);
>
> receiversPanel.updateReceiverTreeInDispatchThread();
> }
> @@ -1834,20 +1833,11 @@
> *
> * @param url
> */
> - private void loadConfigurationUsingPluginClassLoader(final URL
> url) {
> - ClassLoader classLoader =
> PluginClassLoaderFactory.getInstance().getClassLoader();
> - ClassLoader previousTCCL =
> Thread.currentThread().getContextClassLoader();
> + private void loadConfiguration(final URL url) {
>
> if(url!=null) {
> - try {
> - // we temporarily swap the TCCL so that plugins can
> find resources
> - Thread.currentThread().setContextClassLoader(classLoader);
> - JoranConfigurator jc = new JoranConfigurator();
> - jc.doConfigure(url, LogManager.getLoggerRepository());
> - }finally{
> - // now switch it back...
> -
> Thread.currentThread().setContextClassLoader(previousTCCL);
> - }
> + JoranConfigurator jc = new JoranConfigurator();
> + jc.doConfigure(url, LogManager.getLoggerRepository());
> }
> ensureChainsawAppenderHandlerAdded();
> }
>
> Index: src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java
> ===================================================================
> RCS file:
> /home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java,v
>
> retrieving revision 1.9
> diff -u -r1.9 ReceiversHelper.java
> ---
> src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java 24
> Nov 2004 08:17:02 -0000 1.9
> +++
> src/java/org/apache/log4j/chainsaw/receivers/ReceiversHelper.java 12
> Jan 2005 12:18:45 -0000
> @@ -26,7 +26,7 @@
>
> import org.apache.log4j.LogManager;
> import org.apache.log4j.Logger;
> -import org.apache.log4j.chainsaw.plugins.PluginClassLoaderFactory;
> +import org.apache.log4j.helpers.Loader;
>
>
> /**
> @@ -57,23 +57,18 @@
>
> stream = new LineNumberReader(new
> InputStreamReader(url.openStream()));
> String line;
> - // we need the special Classloader, because under Web
> start, optional jars might be local
> - // to this workstation
> - ClassLoader classLoader =
> PluginClassLoaderFactory.getInstance().getClassLoader();
> -
> while ((line = stream.readLine()) != null) {
>
> try {
> if (line.startsWith("#") || (line.length() ==
> 0)) {
> continue;
> }
> - Class receiverClass =
> classLoader.loadClass(line);
> + Class receiverClass = Loader.loadClass(line);
> receiverClassList.add(receiverClass);
> logger.debug("Located known Receiver class " +
> receiverClass.getName());
> } catch (ClassNotFoundException e) {
> - logger.warn("Failed to locate Receiver class:"
> + line);
> - }
> - catch (NoClassDefFoundError e) {
> + logger.warn("Failed to locate Receiver class:
> {}", line);
> + } catch (NoClassDefFoundError e) {
> logger.error("Failed to locate Receiver
> class:" + line + ", looks like a dependent class is missing from the
> classpath", e);
> }
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org