Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On Mon, 2006-02-20 at 21:47 -0800, Craig McClanahan wrote: No, I was not aware of that change ... but does it actually work? Declaring something Serializable is not by itself sufficient if there are transient variables inside the implementation. (On a separate thread on commons-dev, I recommended that there be unit tests to validate this behavior: use a Log instance, serialize it, deserialize it, and use it some more). I believe Serializable is correctly implemented for all standard adapters except possibly AvalonLog [will follow that up on commons-dev]. The deserialized objects correctly locate a fresh underlying log object to forward calls to. Yes there need to be unit tests for this; I'll look for them and add some if not already present. The 1.0.4 release occurred around June 2004 so perhaps there are a few containers out there capable of running MyFaces but which are still on JCL 1.0.3. In addition there might be a few custom Log adapters around which *might* not implement Serializable. However in view of the complicated/ugly workaround for non-serializable Log implementations, it seems ok to me for MyFaces to just declare a dependency on JCL 1.0.4. If people don't comply then there is a very obvious NotSerializableException: logAdapterClassName message generated. The fix is then to upgrade to 1.0.4 (which is binary compatible with earlier releases) or a later release. Comments/Opinions? +1 on declaring a dependency on Commons Logging 1.0.4 ... anything prior to that is just asking for trouble. I use 1.0.4 exclusively for eveything that has a C-L dependency ... works fine, lasts a long time :-). So the conclusion seems to be that if/when using commons-logging in MyFaces, use: private Log foo = LogFactory.getLog(foo); everywhere except in short-lived and frequently-created objects. In those cases, either call LogFactory.getLog as needed or have some longer-lived object store the Log object. For StateHolder classes, things just work; the logger isn't saved in saveState; it gets recreated in the new component and that works. For Serializable classes, the Log object handles its own serialization correctly. No need for transient, no need for any other special hacks. MyFaces does need to declare a dependency on commons-logging 1.0.4 (not earlier versions) in order for things to work this simply. Release 1.0.4 was in June 2004. Do NOT use static Log anywhere. Is this all ok? Cheers, Simon
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
+1 for dependency on commons-logging 1.0.4 BTW, Stan (Silvert), how do you solve these logging issues in JBoss? AFAIK, JBoss has only one central log4j configuration. However, is there a way to config logging per eapp or webapp? If yes, how does JBoss address those issues with shared classes? Thanks, Manfred On 2/21/06, Simon Kitching [EMAIL PROTECTED] wrote: On Mon, 2006-02-20 at 21:47 -0800, Craig McClanahan wrote: No, I was not aware of that change ... but does it actually work? Declaring something Serializable is not by itself sufficient if there are transient variables inside the implementation. (On a separate thread on commons-dev, I recommended that there be unit tests to validate this behavior: use a Log instance, serialize it, deserialize it, and use it some more). I believe Serializable is correctly implemented for all standard adapters except possibly AvalonLog [will follow that up on commons-dev]. The deserialized objects correctly locate a fresh underlying log object to forward calls to. Yes there need to be unit tests for this; I'll look for them and add some if not already present. The 1.0.4 release occurred around June 2004 so perhaps there are a few containers out there capable of running MyFaces but which are still on JCL 1.0.3. In addition there might be a few custom Log adapters around which *might* not implement Serializable. However in view of the complicated/ugly workaround for non-serializable Log implementations, it seems ok to me for MyFaces to just declare a dependency on JCL 1.0.4. If people don't comply then there is a very obvious NotSerializableException: logAdapterClassName message generated. The fix is then to upgrade to 1.0.4 (which is binary compatible with earlier releases) or a later release. Comments/Opinions? +1 on declaring a dependency on Commons Logging 1.0.4 ... anything prior to that is just asking for trouble. I use 1.0.4 exclusively for eveything that has a C-L dependency ... works fine, lasts a long time :-). So the conclusion seems to be that if/when using commons-logging in MyFaces, use: private Log foo = LogFactory.getLog(foo); everywhere except in short-lived and frequently-created objects. In those cases, either call LogFactory.getLog as needed or have some longer-lived object store the Log object. For StateHolder classes, things just work; the logger isn't saved in saveState; it gets recreated in the new component and that works. For Serializable classes, the Log object handles its own serialization correctly. No need for transient, no need for any other special hacks. MyFaces does need to declare a dependency on commons-logging 1.0.4 (not earlier versions) in order for things to work this simply. Release 1.0.4 was in June 2004. Do NOT use static Log anywhere. Is this all ok? Cheers, Simon
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Yes +1 for 1.0.4. Thanks for explaining all of this. On 2/21/06, Manfred Geiler [EMAIL PROTECTED] wrote: +1 for dependency on commons-logging 1.0.4 BTW, Stan (Silvert), how do you solve these logging issues in JBoss? AFAIK, JBoss has only one central log4j configuration. However, is there a way to config logging per eapp or webapp? If yes, how does JBoss address those issues with shared classes? Thanks, Manfred On 2/21/06, Simon Kitching [EMAIL PROTECTED] wrote: On Mon, 2006-02-20 at 21:47 -0800, Craig McClanahan wrote: No, I was not aware of that change ... but does it actually work? Declaring something Serializable is not by itself sufficient if there are transient variables inside the implementation. (On a separate thread on commons-dev, I recommended that there be unit tests to validate this behavior: use a Log instance, serialize it, deserialize it, and use it some more). I believe Serializable is correctly implemented for all standard adapters except possibly AvalonLog [will follow that up on commons-dev]. The deserialized objects correctly locate a fresh underlying log object to forward calls to. Yes there need to be unit tests for this; I'll look for them and add some if not already present. The 1.0.4 release occurred around June 2004 so perhaps there are a few containers out there capable of running MyFaces but which are still on JCL 1.0.3. In addition there might be a few custom Log adapters around which *might* not implement Serializable. However in view of the complicated/ugly workaround for non-serializable Log implementations, it seems ok to me for MyFaces to just declare a dependency on JCL 1.0.4. If people don't comply then there is a very obvious NotSerializableException: logAdapterClassName message generated. The fix is then to upgrade to 1.0.4 (which is binary compatible with earlier releases) or a later release. Comments/Opinions? +1 on declaring a dependency on Commons Logging 1.0.4 ... anything prior to that is just asking for trouble. I use 1.0.4 exclusively for eveything that has a C-L dependency ... works fine, lasts a long time :-). So the conclusion seems to be that if/when using commons-logging in MyFaces, use: private Log foo = LogFactory.getLog(foo); everywhere except in short-lived and frequently-created objects. In those cases, either call LogFactory.getLog as needed or have some longer-lived object store the Log object. For StateHolder classes, things just work; the logger isn't saved in saveState; it gets recreated in the new component and that works. For Serializable classes, the Log object handles its own serialization correctly. No need for transient, no need for any other special hacks. MyFaces does need to declare a dependency on commons-logging 1.0.4 (not earlier versions) in order for things to work this simply. Release 1.0.4 was in June 2004. Do NOT use static Log anywhere. Is this all ok? Cheers, Simon
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote: On Sun, 2006-02-19 at 22:33 -0800, Adam Winer wrote: Weee if you implement StateHolder, this isn't an issue. The public no-arg constructor will be used, variable initializer expressions will run, etc. If you implement Serializable instead, then Craig's totally right - transient variables will not be re-initialized. You can deal with this by adding the log() method, or by providing an implementation of: private void readObject(ObjectInputStream in) ... which can re-initialize any transient values. Thanks for the info. I am *so* thankful that java.util.logging doesn't force any of this pain on its users. Well, it does this by not providing the ability for EITHER of: (a) logging implementations in webapps, or (b) logging configurations in webapps Not exactly true: it's not enabled out-of-the-box (which I agree is lousy), but a solid JDK 1.4 application server can enable it. I know OC4J does. The moment a non-standard java.util.logging implementation tries to do either, exactly this problem reappears. And then the problem is not documented in the API the user is using! The problem only reappears if someone other than the application server tries to provide the feature. If the app server provides it, it is fully in control of the classloaders used, and can ensure that the Logger classes in use are loaded by the parent classloader. (The fact that there is one and only one LogManager necessarily puts the onus on the app server.) Pressure should be applied to app servers to get this important feature right. Tomcat doesn't, AFAIK, which is basically just a big missing feature of Tomcat, not a missing featurer of java.util.logging. It's easy to avoid bugs by not providing functionality. I can provide you with a totally bug-free web presentation tier in 30 seconds -- provided all it ever does is display hello world. And actually, it's not a bug - it's an architectural issue due to (IMO) the poor Java classloader model. If static variables weren't shared between webapps then the problem wouldn't occur. Ecch, the sheer ugliness of a container framework that causes or allows that kind of interaction between supposedly independent component applications! It's not a bug is of scant consolation to users (like myself) who have to swear at their computer when it's throwing ClassCastExceptions because code they don't control didn't get the memo on not creating static Log instances. IIRC, you have a personal stake in this issue, but the plain truth is that there is no war anymore - java.util.logging won by Sun's fiat. Technical superiority is not the ultimate arbiter. For MyFaces at this time, it's a non-issue at least for the API and IMPL core, since JSF 1.1 needs to support JDK 1.3, and that makes java.util.logging a non-starter for those parts of MyFaces. (And, IMO, it'd be odd for Tomahawk to have a different JDK requirement than the JSF impl.) Regards, Adam
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On Mon, 2006-02-20 at 11:51 -0800, Adam Winer wrote: IIRC, you have a personal stake in this issue, but the plain truth is that there is no war anymore - java.util.logging won by Sun's fiat. Technical superiority is not the ultimate arbiter. Yes, I am a commons-logging committer. That doesn't mean I believe it's perfect; I work on it because I need it, and because it *does* have flaws that need work. I don't have any interest in defending JCL against true complaints; the ClassCastException problems experienced with 1.0.4 and earlier are an example (though one that is at least partially caused by what I consider to be bad container design in many cases). The recent problems pointed out about Log not being Serializable and the implications thereof are another; that's truly ugly. However it appears that no-one has ever bothered to point this out on the email lists or bugzilla. I've reported this to the right lists, and there may well be a fix in the upcoming release. However there is a lot of truly incorrect information floating around about JCL; one person makes a false claim, another quotes it, then it becomes accepted as truth despite the fact that it is complete garbage. Unfortunately a number of articles linked to from the log4j website fall into this category. I do try to correct such claims when I see them because having false info does no-one any favours. BTW, I also agree that j.u.logging will eventually be *the* logging API, and I'll be glad when that day comes and JCL can be retired. Logging definitely should be in the language core. It's a shame Sun failed to consult more widely with the community on design but nevertheless j.u.l is *useable*. However there's a whole lot of people on pre-1.4 JVMs, and that's going to continue to be the case for a number of years. I am *so* thankful that java.util.logging doesn't force any of this pain on its users. Well, it does this by not providing the ability for EITHER of: (a) logging implementations in webapps, or (b) logging configurations in webapps Not exactly true: it's not enabled out-of-the-box (which I agree is lousy), but a solid JDK 1.4 application server can enable it. I know OC4J does. The moment a non-standard java.util.logging implementation tries to do either, exactly this problem reappears. And then the problem is not documented in the API the user is using! The problem only reappears if someone other than the application server tries to provide the feature. If the app server provides it, it is fully in control of the classloaders used, and can ensure that the Logger classes in use are loaded by the parent classloader. (The fact that there is one and only one LogManager necessarily puts the onus on the app server.) I would *really* be interested in more info on how OC4J can do this. It sure seems impossible to me. The static field on the shared class points to exactly ONE Log object, yes? And that Log object must have been configured from exactly ONE of: (a) config info in webapp A (b) config info in webapp B (c) config info in the container So how is it possible for calls from webappA and webappB to both behave correctly if they are accessing a shared class with a static Log member? NB: The Log object could actually be a proxy where each call to each method looks up the TCCL then dispatches to an appropriate *real* Log object, but that has horrible performance implications. Note that we are *not* talking about classloaders at all here, except that the TCCL is used as a key for different logging configurations. ClassCastException issues are a different problem. Pressure should be applied to app servers to get this important feature right. Tomcat doesn't, AFAIK, which is basically just a big missing feature of Tomcat, not a missing featurer of java.util.logging. If you can give a hint on how it's possible I'd be happy to work to see this gets into tomcat. It's easy to avoid bugs by not providing functionality. I can provide you with a totally bug-free web presentation tier in 30 seconds -- provided all it ever does is display hello world. And actually, it's not a bug - it's an architectural issue due to (IMO) the poor Java classloader model. If static variables weren't shared between webapps then the problem wouldn't occur. Ecch, the sheer ugliness of a container framework that causes or allows that kind of interaction between supposedly independent component applications! It's not a bug is of scant consolation to users (like myself) who have to swear at their computer when it's throwing ClassCastExceptions because code they don't control didn't get the memo on not creating static Log instances. Agreed. However the JCL 1.1 release has a solution for this; it's possible to disable tccl classloading completely. Of course that disables useful functionality (reducing things to the default j.u.logging level) but it definitely fixes any
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On 2/20/06, Simon Kitching [EMAIL PROTECTED] wrote: On Mon, 2006-02-20 at 11:51 -0800, Adam Winer wrote: IIRC, you have a personal stake in this issue, but the plain truth is that there is no war anymore - java.util.logging won by Sun's fiat. Technical superiority is not the ultimate arbiter. Yes, I am a commons-logging committer. That doesn't mean I believe it's perfect; I work on it because I need it, and because it *does* have flaws that need work. I don't have any interest in defending JCL against true complaints; the ClassCastException problems experienced with 1.0.4 and earlier are an example (though one that is at least partially caused by what I consider to be bad container design in many cases). The recent problems pointed out about Log not being Serializable and the implications thereof are another; that's truly ugly. However it appears that no-one has ever bothered to point this out on the email lists or bugzilla. I've reported this to the right lists, and there may well be a fix in the upcoming release. However there is a lot of truly incorrect information floating around about JCL; one person makes a false claim, another quotes it, then it becomes accepted as truth despite the fact that it is complete garbage. Unfortunately a number of articles linked to from the log4j website fall into this category. I do try to correct such claims when I see them because having false info does no-one any favours. BTW, I also agree that j.u.logging will eventually be *the* logging API, and I'll be glad when that day comes and JCL can be retired. Logging definitely should be in the language core. It's a shame Sun failed to consult more widely with the community on design but nevertheless j.u.l is *useable*. However there's a whole lot of people on pre-1.4 JVMs, and that's going to continue to be the case for a number of years. I am *so* thankful that java.util.logging doesn't force any of this pain on its users. Well, it does this by not providing the ability for EITHER of: (a) logging implementations in webapps, or (b) logging configurations in webapps Not exactly true: it's not enabled out-of-the-box (which I agree is lousy), but a solid JDK 1.4 application server can enable it. I know OC4J does. The moment a non-standard java.util.logging implementation tries to do either, exactly this problem reappears. And then the problem is not documented in the API the user is using! The problem only reappears if someone other than the application server tries to provide the feature. If the app server provides it, it is fully in control of the classloaders used, and can ensure that the Logger classes in use are loaded by the parent classloader. (The fact that there is one and only one LogManager necessarily puts the onus on the app server.) I would *really* be interested in more info on how OC4J can do this. It sure seems impossible to me. Hrm - I think I have to eat some crow here; Just checked, and OC4J *might* be able to do it, but the included configuration webapp only allows configuration at a global level, not per-webapp. So it almost certainly can't. The static field on the shared class points to exactly ONE Log object, yes? And that Log object must have been configured from exactly ONE of: (a) config info in webapp A (b) config info in webapp B (c) config info in the container So how is it possible for calls from webappA and webappB to both behave correctly if they are accessing a shared class with a static Log member? Shared class or not, it can't, not without proxying or additional lookup effort. NB: The Log object could actually be a proxy where each call to each method looks up the TCCL then dispatches to an appropriate *real* Log object, but that has horrible performance implications. Indeed, it would; the absolute best I could imagine is that the container configures the Log to have the most verbose level requested by any webapp, then have a Filter check the TCCL and trim out ones that don't apply to the current webapp. This *might* be acceptable, but I can easily imagine circumstances where it would not - crank up insanely verbose levels on a test app while leaving up the real app that's getting normal test loads. You'd be churning out LogRecords, though at least the log files wouldn't get flooded. Note that we are *not* talking about classloaders at all here, except that the TCCL is used as a key for different logging configurations. ClassCastException issues are a different problem. Yep. TCCL; alternatively, stash the key on a ThreadLocal, which is probably faster still. We use ThreadLocals quite a bit, and I keep waiting for them to show up in any profile as a bottleneck - they never have yet. Pressure should be applied to app servers to get this important feature right. Tomcat doesn't, AFAIK, which is basically just a big missing feature of Tomcat, not a
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Hi Craig,
On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote:
There *is* a JSF-specific consideration to think about, if you have
classes that implement StateHolder (like a UIComponent
implementation). Log instances will generally *not* be serializable,
so you will need to deal specially with them in saveState() and
restoreState() methods. The simplest thing is to just not save them,
and do a new operation again in restoreState().
Along the same lines, if your class implements Serializable, you will
need to mark the instance variable transient. I've started using the
following pattern in my Serializable classes, which would work inside
a StateHolder as well:
private transient Log log = null;
private Log log() {
if (log == null) {
log = LogFactory.getLog(...);
}
return log;
}
and a typical call looks like:
if (log().isDebugEnabled()) {
log().debug(...);
}
I've checked, and all the standard commons-logging Log adapter
implementations *are* Serializable (Log4J, JDK14, LogKit, Simple, NoOp).
So AIUI there is no need to declare Log members transient; they should
get serialized and deserialized along with the owning class fine. No
log() method would then be required or any other special handling (other
than ensuring the member is NOT static!).
Serializable support for Log implementations appears to have been
implemented in the JCL 1.0.4 release, ie in JCL 1.0.3 and earlier the
Log implementations were not Serializable. Craig: maybe you weren't
aware of this change?
The 1.0.4 release occurred around June 2004 so perhaps there are a few
containers out there capable of running MyFaces but which are still on
JCL 1.0.3. In addition there might be a few custom Log adapters around
which *might* not implement Serializable.
However in view of the complicated/ugly workaround for non-serializable
Log implementations, it seems ok to me for MyFaces to just declare a
dependency on JCL 1.0.4. If people don't comply then there is a very
obvious NotSerializableException: logAdapterClassName message
generated. The fix is then to upgrade to 1.0.4 (which is binary
compatible with earlier releases) or a later release.
Comments/Opinions?
Cheers,
Simon
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On 2/20/06, Simon Kitching [EMAIL PROTECTED] wrote:
Hi Craig,On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote: There *is* a JSF-specific consideration to think about, if you have classes that implement StateHolder (like a UIComponent implementation).Log instances will generally *not* be serializable,
so you will need to deal specially with them in saveState() and restoreState() methods.The simplest thing is to just not save them, and do a new operation again in restoreState().
Along the same lines, if your class implements Serializable, you will need to mark the instance variable transient.I've started using the following pattern in my Serializable classes, which would work inside
a StateHolder as well: private transient Log log = null; private Log log() { if (log == null) { log = LogFactory.getLog(...); }
return log; } and a typical call looks like: if (log().isDebugEnabled()) {log().debug(...); }I've checked, and all the standard commons-logging Log adapter
implementations *are* Serializable (Log4J, JDK14, LogKit, Simple, NoOp).So AIUI there is no need to declare Log members transient; they shouldget serialized and deserialized along with the owning class fine. No
log() method would then be required or any other special handling (otherthan ensuring the member is NOT static!).Serializable support for Log implementations appears to have beenimplemented in the JCL
1.0.4 release, ie in JCL 1.0.3 and earlier theLog implementations were not Serializable. Craig: maybe you weren'taware of this change?No, I was not aware of that change ... but does it actually work? Declaring something Serializable is not by itself sufficient if there are transient variables inside the implementation. (On a separate thread on commons-dev, I recommended that there be unit tests to validate this behavior: use a Log instance, serialize it, deserialize it, and use it some more).
The 1.0.4 release occurred around June 2004 so perhaps there are a fewcontainers out there capable of running MyFaces but which are still on
JCL 1.0.3. In addition there might be a few custom Log adapters aroundwhich *might* not implement Serializable.However in view of the complicated/ugly workaround for non-serializableLog implementations, it seems ok to me for MyFaces to just declare a
dependency on JCL 1.0.4. If people don't comply then there is a veryobvious NotSerializableException: logAdapterClassName messagegenerated. The fix is then to upgrade to 1.0.4 (which is binarycompatible with earlier releases) or a later release.
Comments/Opinions?+1 on declaring a dependency on Commons Logging 1.0.4 ... anything prior to that is just asking for trouble. I use 1.0.4 exclusively for eveything that has a C-L dependency ... works fine, lasts a long time :-).
Cheers,SimonCraig
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
ok - so that means we'll need to drop static?
won't that cause performance problems?
regards,
Martin
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 00:46 +, [EMAIL PROTECTED] wrote:
Author: mmarinschek
Date: Sat Feb 18 16:46:18 2006
New Revision: 378805
URL: http://svn.apache.org/viewcvs?rev=378805view=rev
Log:
minor changes in application-factory, fixed readOnly referral in
HtmlRendererUtils
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
URL:
http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805r1=378804r2=378805view=diff
==
---
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
(original)
+++
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Sat Feb 18 16:46:18 2006
@@ -29,12 +29,14 @@
public class ApplicationFactoryImpl
extends ApplicationFactory
{
-private static final Log log =
LogFactory.getLog(ApplicationFactory.class);
+private static final Log log =
LogFactory.getLog(ApplicationFactoryImpl.class);
Just a warning to all developers: when using commons-logging in a
library, STATIC fields must **NOT** be used to store Log objects.
The problem is that the class may be called with various thread-context
classloaders (TCCL) set. However the class is only ever loaded once.
If the Log object is static, then it is initialised using the TCCL of
the very first webapp to use it (ie which forces the class to be loaded)
- including picking up config info from that TCCL. All other webapps
will then use the same Log object - which is not healthy.
Yes this is a nuisance, but it's absolutely necessary to avoid storing
Log instances in static fields for libraries like MyFaces.
Regards,
Simon
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 00:46 +, [EMAIL PROTECTED] wrote:
Author: mmarinschek
Date: Sat Feb 18 16:46:18 2006
New Revision: 378805
URL: http://svn.apache.org/viewcvs?rev=378805view=rev
Log:
minor changes in application-factory, fixed readOnly referral in
HtmlRendererUtils
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
URL:
http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805r1=378804r2=378805view=diff
==
---
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
(original)
+++
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Sat Feb 18 16:46:18 2006
@@ -29,12 +29,14 @@
public class ApplicationFactoryImpl
extends ApplicationFactory
{
-private static final Log log =
LogFactory.getLog(ApplicationFactory.class);
+private static final Log log =
LogFactory.getLog(ApplicationFactoryImpl.class);
Just a warning to all developers: when using commons-logging in a
library, STATIC fields must **NOT** be used to store Log objects.
The problem is that the class may be called with various thread-context
classloaders (TCCL) set. However the class is only ever loaded once.
Never heard of that. Simon are you really sure?
I think the opposite is true. Containers tend to have Classloaders
that all have there own set of loaded classes. So you get problems
when your applications Classloader holds some Classes that the
container itself has already loaded. Then you get such things like
incomptible class Exceptions and NoClassDefFound when a class in
your webapp is linked to another class in your app but the Loader
finds the containers class first.
Manfred
If the Log object is static, then it is initialised using the TCCL of
the very first webapp to use it (ie which forces the class to be loaded)
- including picking up config info from that TCCL. All other webapps
will then use the same Log object - which is not healthy.
Yes this is a nuisance, but it's absolutely necessary to avoid storing
Log instances in static fields for libraries like MyFaces.
Regards,
Simon
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On Sun, 2006-02-19 at 11:36 +0100, Martin Marinschek wrote:
ok - so that means we'll need to drop static?
Yes. In fact, because MyFaces libs are often placed in a shared
classpath, static should be avoided in almost all cases I expect.
won't that cause performance problems?
Calling LogFactory.getLog is *reasonably* efficient.
The impact is trivial for long-lived objects like
ApplicationFactoryImpl. I wouldn't worry about performance for things
like UIComponent objects either, which have a lifetime of a request.
If there are very short-lived objects which are frequently created, then
it might be worth considering either:
(a) only fetching a Log object if logging is actually needed, ie
if (some condition) {
Log log = LogFactory.getLog(Foo.class);
log.warn();
}
(b) using the Log object stored on some other component.
longLivedObject.getXYZLog().debug()
Of course this works only when there is some suitable
longer-lived object that is easily accessable from the
short-lived object that wants to do logging.
Cheers,
Simon
regards,
Martin
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 00:46 +, [EMAIL PROTECTED] wrote:
Author: mmarinschek
Date: Sat Feb 18 16:46:18 2006
New Revision: 378805
URL: http://svn.apache.org/viewcvs?rev=378805view=rev
Log:
minor changes in application-factory, fixed readOnly referral in
HtmlRendererUtils
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
URL:
http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805r1=378804r2=378805view=diff
==
---
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
(original)
+++
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Sat Feb 18 16:46:18 2006
@@ -29,12 +29,14 @@
public class ApplicationFactoryImpl
extends ApplicationFactory
{
-private static final Log log =
LogFactory.getLog(ApplicationFactory.class);
+private static final Log log =
LogFactory.getLog(ApplicationFactoryImpl.class);
Just a warning to all developers: when using commons-logging in a
library, STATIC fields must **NOT** be used to store Log objects.
The problem is that the class may be called with various thread-context
classloaders (TCCL) set. However the class is only ever loaded once.
If the Log object is static, then it is initialised using the TCCL of
the very first webapp to use it (ie which forces the class to be loaded)
- including picking up config info from that TCCL. All other webapps
will then use the same Log object - which is not healthy.
Yes this is a nuisance, but it's absolutely necessary to avoid storing
Log instances in static fields for libraries like MyFaces.
Regards,
Simon
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On Sun, 2006-02-19 at 20:27 +0100, Manfred Geiler wrote: On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote: Just a warning to all developers: when using commons-logging in a library, STATIC fields must **NOT** be used to store Log objects. The problem is that the class may be called with various thread-context classloaders (TCCL) set. However the class is only ever loaded once. Never heard of that. Simon are you really sure? I think the opposite is true. Containers tend to have Classloaders that all have there own set of loaded classes. So you get problems when your applications Classloader holds some Classes that the container itself has already loaded. Then you get such things like incomptible class Exceptions and NoClassDefFound when a class in your webapp is linked to another class in your app but the Loader finds the containers class first. Yes, I'm really sure. Here's the relevant paragraph from the user-guide for the upcoming 1.1 release [written by me]: http://jakarta.apache.org/commons/logging/guide.html#Developing%20With% 20JCL This is not about class compatibility issues; using instance members rather than class members doesn't change the classpath/classloader/etc. It's about the fact that when the field is static, there is only *one* Log instance for that class but multiple webapps are using it. Once constructed, the Log object simply points directly to a *configured* object within the underlying concrete logging library. That underlying object therefore either: (a) has no webapp-specific state (ie is initialised via config info associated with the container not a TCCL) (b) has the state of the first webapp that initialised it (ie is initialised via config info located via the TCCL active when the Log object was created). Option (a) occurs if the first call to the class is from the container itself, or if the concrete logging library used is in a shared classpath, or if TCCL-based configuration is not enabled in commons-logging. In this case, using a static Log is actually ok. Option (b) occurs when TCCL-based configuration is enabled, and a webapp is the first to access that class. The latter case is the usual one, resulting in logging output generated *as a result of webapp B calling MyFaces* ending up in the logfiles for *webapp A*. Not good. Any logging library that attempts to direct output from shared classes into the logfile of the webapp *it is running on behalf of* will have this issue. Log4j doesn't have this problem by default, as it doesn't attempt to put output from shared classes into webapp-specific logfiles. However it *does* have an optional mechanism to do this: the RepositorySelector. http://www.qos.ch/logging/sc.jsp If the Contextual Repository Selector approach is used as documented in the above page, then exactly the same benefits and problems occur in log4j as in commons-logging; logging by shared classes goes into the relevant webapp logfile [good] but static Log objects in shared classes can output to the wrong logfile [bad]. Avoiding static Log instances in potentially shared classes is the only reliable solution. And as I mentioned in the earlier email, static fields in shared classes should be regarded with great suspicion in *all* cases, not just logging! Regards, Simon
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Hmm ... then this is a good candidate for the most often used anti-pattern. Is there anything in checkstyle for it? Dennis Byrne -Original Message- From: Martin Marinschek [mailto:[EMAIL PROTECTED] Sent: Sunday, February 19, 2006 04:57 PM To: 'MyFaces Development', [EMAIL PROTECTED] Subject: Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java wo-ow. it's great to learn a new thing every day ;) regards, Martin On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote: On Sun, 2006-02-19 at 20:27 +0100, Manfred Geiler wrote: On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote: Just a warning to all developers: when using commons-logging in a library, STATIC fields must **NOT** be used to store Log objects. The problem is that the class may be called with various thread-context classloaders (TCCL) set. However the class is only ever loaded once. Never heard of that. Simon are you really sure? I think the opposite is true. Containers tend to have Classloaders that all have there own set of loaded classes. So you get problems when your applications Classloader holds some Classes that the container itself has already loaded. Then you get such things like incomptible class Exceptions and NoClassDefFound when a class in your webapp is linked to another class in your app but the Loader finds the containers class first. Yes, I'm really sure. Here's the relevant paragraph from the user-guide for the upcoming 1.1 release [written by me]: http://jakarta.apache.org/commons/logging/guide.html#Developing%20With% 20JCL This is not about class compatibility issues; using instance members rather than class members doesn't change the classpath/classloader/etc. It's about the fact that when the field is static, there is only *one* Log instance for that class but multiple webapps are using it. Once constructed, the Log object simply points directly to a *configured* object within the underlying concrete logging library. That underlying object therefore either: (a) has no webapp-specific state (ie is initialised via config info associated with the container not a TCCL) (b) has the state of the first webapp that initialised it (ie is initialised via config info located via the TCCL active when the Log object was created). Option (a) occurs if the first call to the class is from the container itself, or if the concrete logging library used is in a shared classpath, or if TCCL-based configuration is not enabled in commons-logging. In this case, using a static Log is actually ok. Option (b) occurs when TCCL-based configuration is enabled, and a webapp is the first to access that class. The latter case is the usual one, resulting in logging output generated *as a result of webapp B calling MyFaces* ending up in the logfiles for *webapp A*. Not good. Any logging library that attempts to direct output from shared classes into the logfile of the webapp *it is running on behalf of* will have this issue. Log4j doesn't have this problem by default, as it doesn't attempt to put output from shared classes into webapp-specific logfiles. However it *does* have an optional mechanism to do this: the RepositorySelector. http://www.qos.ch/logging/sc.jsp If the Contextual Repository Selector approach is used as documented in the above page, then exactly the same benefits and problems occur in log4j as in commons-logging; logging by shared classes goes into the relevant webapp logfile [good] but static Log objects in shared classes can output to the wrong logfile [bad]. Avoiding static Log instances in potentially shared classes is the only reliable solution. And as I mentioned in the earlier email, static fields in shared classes should be regarded with great suspicion in *all* cases, not just logging! Regards, Simon -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces
logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote:
Simon,
Could you do me a favor and publicize this in the Struts community as
well? The framework code there is littered with static log instances
to.
Will do.
You might also want to add some notes related to using Log instances
in Serializable classes (see below).
Will do that also.
MyFaces folks,
There *is* a JSF-specific consideration to think about, if you have
classes that implement StateHolder (like a UIComponent
implementation). Log instances will generally *not* be serializable,
so you will need to deal specially with them in saveState() and
restoreState() methods. The simplest thing is to just not save them,
and do a new operation again in restoreState().
Sorry but I don't understand. Why won't the normal approach work?
public class SomeComponent {
private Log log = LogFactory.getLog(SomeComponent.class);
...
}
AIUI, the log object won't be saved in the saveState method, but it will
be recreated nicely during the RestoreView phase when a new instance is
created for the state to be restored into.
Along the same lines, if your class implements Serializable, you will
need to mark the instance variable transient. I've started using the
following pattern in my Serializable classes, which would work inside
a StateHolder as well:
private transient Log log = null;
private Log log() {
if (log == null) {
log = LogFactory.getLog(...);
}
return log;
}
and a typical call looks like:
if (log().isDebugEnabled()) {
log().debug(...);
}
Ok, transient is needed here. But apart from that why won't the standard
approach work?
public class SomeThing implements Serializable {
private transient Log log = LogFactory.getLog(SomeThing.class);
...
if (log.isDebugEnabled()) {
log.debug(...);
}
}
Doesn't the log object get recreated when the deserialization occurs?
The log() method is quite a lot of boilerplate, and also imposes an
additional method call for every isDebugEnabled() call. [The extra call
inside the guard is not really relevant as output *is* going to occur at
this point which greatly outweighs the price of the method call].
Cheers,
Simon
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote: Simon, Could you do me a favor and publicize this in the Struts community as well?The framework code there is littered with static log instances
to.Will do.You might also want to add some notes related to using Log instances in Serializable classes (see below).Will do that also. MyFaces folks,
There *is* a JSF-specific consideration to think about, if you have classes that implement StateHolder (like a UIComponent implementation).Log instances will generally *not* be serializable,
so you will need to deal specially with them in saveState() and restoreState() methods.The simplest thing is to just not save them, and do a new operation again in restoreState().
Sorry but I don't understand. Why won't the normal approach work?public class SomeComponent {private Log log = LogFactory.getLog(SomeComponent.class);...}AIUI, the log object won't be saved in the saveState method, but it will
be recreated nicely during the RestoreView phase when a new instance iscreated for the state to be restored into. Along the same lines, if your class implements Serializable, you will need to mark the instance variable transient.I've started using the
following pattern in my Serializable classes, which would work inside a StateHolder as well: private transient Log log = null; private Log log() { if (log == null) {
log = LogFactory.getLog(...); } return log; } and a typical call looks like: if (log().isDebugEnabled()) {log().debug(...);
}Ok, transient is needed here. But apart from that why won't the standardapproach work?public class SomeThing implements Serializable {private transient Log log = LogFactory.getLog
(SomeThing.class);...if (log.isDebugEnabled()) {log.debug(...);}}Doesn't the log object get recreated when the deserialization occurs?No, AIUI. When an object is deserialized, it does *not* execute the variable initializer expressions. Since it was declared transient, there's no state for that object to be restored.
The log() method is quite a lot of boilerplate, and also imposes anadditional method call for every isDebugEnabled() call. [The extra call
inside the guard is not really relevant as output *is* going to occur atthis point which greatly outweighs the price of the method call].You can reduce that by one call by protecting only the conditional _expression_, because if you get inside you know there's an object ... but that means you are relying on the implicit contract between the log instance variable and the log() method.
Cheers,SimonCraig
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
Weee if you implement StateHolder, this isn't an issue.
The public no-arg constructor will be used, variable initializer
expressions will run, etc.
If you implement Serializable instead, then Craig's totally right -
transient variables will not be re-initialized. You can deal with
this by adding the log() method, or by providing an implementation
of:
private void readObject(ObjectInputStream in)
... which can re-initialize any transient values.
I am *so* thankful that java.util.logging doesn't force any of
this pain on its users.
-- Adam
On 2/19/06, Craig McClanahan [EMAIL PROTECTED] wrote:
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote:
Simon,
Could you do me a favor and publicize this in the Struts community as
well? The framework code there is littered with static log instances
to.
Will do.
You might also want to add some notes related to using Log instances
in Serializable classes (see below).
Will do that also.
MyFaces folks,
There *is* a JSF-specific consideration to think about, if you have
classes that implement StateHolder (like a UIComponent
implementation). Log instances will generally *not* be serializable,
so you will need to deal specially with them in saveState() and
restoreState() methods. The simplest thing is to just not save them,
and do a new operation again in restoreState().
Sorry but I don't understand. Why won't the normal approach work?
public class SomeComponent {
private Log log = LogFactory.getLog(SomeComponent.class);
...
}
AIUI, the log object won't be saved in the saveState method, but it will
be recreated nicely during the RestoreView phase when a new instance is
created for the state to be restored into.
Along the same lines, if your class implements Serializable, you will
need to mark the instance variable transient. I've started using the
following pattern in my Serializable classes, which would work inside
a StateHolder as well:
private transient Log log = null;
private Log log() {
if (log == null) {
log = LogFactory.getLog(...);
}
return log;
}
and a typical call looks like:
if (log().isDebugEnabled()) {
log().debug(...);
}
Ok, transient is needed here. But apart from that why won't the standard
approach work?
public class SomeThing implements Serializable {
private transient Log log = LogFactory.getLog (SomeThing.class);
...
if (log.isDebugEnabled()) {
log.debug(...);
}
}
Doesn't the log object get recreated when the deserialization occurs?
No, AIUI. When an object is deserialized, it does *not* execute the
variable initializer expressions. Since it was declared transient, there's
no state for that object to be restored.
The log() method is quite a lot of boilerplate, and also imposes an
additional method call for every isDebugEnabled() call. [The extra call
inside the guard is not really relevant as output *is* going to occur at
this point which greatly outweighs the price of the method call].
You can reduce that by one call by protecting only the conditional
expression, because if you get inside you know there's an object ... but
that means you are relying on the implicit contract between the log instance
variable and the log() method.
Cheers,
Simon
Craig
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On Sun, 2006-02-19 at 22:33 -0800, Adam Winer wrote: Weee if you implement StateHolder, this isn't an issue. The public no-arg constructor will be used, variable initializer expressions will run, etc. If you implement Serializable instead, then Craig's totally right - transient variables will not be re-initialized. You can deal with this by adding the log() method, or by providing an implementation of: private void readObject(ObjectInputStream in) ... which can re-initialize any transient values. Thanks for the info. I am *so* thankful that java.util.logging doesn't force any of this pain on its users. Well, it does this by not providing the ability for EITHER of: (a) logging implementations in webapps, or (b) logging configurations in webapps The moment a non-standard java.util.logging implementation tries to do either, exactly this problem reappears. And then the problem is not documented in the API the user is using! It's easy to avoid bugs by not providing functionality. I can provide you with a totally bug-free web presentation tier in 30 seconds -- provided all it ever does is display hello world. And actually, it's not a bug - it's an architectural issue due to (IMO) the poor Java classloader model. If static variables weren't shared between webapps then the problem wouldn't occur. Ecch, the sheer ugliness of a container framework that causes or allows that kind of interaction between supposedly independent component applications! Regards, Simon
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
On 2/19/06, Martin Marinschek [EMAIL PROTECTED] wrote:
Indeed.this really means I'd be willing to switch to JDK logging. What wasthe reason again we couldn't do that?If we are willing to live with a JDK 1.4 or later restriction, no reason at all. That, however, would seem to be an issue for some current users.
regards,MartinCraig
On 2/20/06, Adam Winer [EMAIL PROTECTED] wrote: Weeeif you implement StateHolder, this isn't an issue. The public no-arg constructor will be used, variable initializer
expressions will run, etc. If you implement Serializable instead, then Craig's totally right - transient variables will not be re-initialized. You can deal with this by adding the log() method, or by providing an implementation
of:private void readObject(ObjectInputStream in) ... which can re-initialize any transient values. I am *so* thankful that java.util.logging doesn't force any of
this pain on its users. -- Adam On 2/19/06, Craig McClanahan [EMAIL PROTECTED] wrote:
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote: On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote: Simon,
Could you do me a favor and publicize this in the Struts community aswell?The framework code there is littered with static log instancesto.
Will do. You might also want to add some notes related to using Log instancesin Serializable classes (see below).
Will do that also. MyFaces folks, There *is* a JSF-specific consideration to think about, if you have
classes that implement StateHolder (like a UIComponentimplementation).Log instances will generally *not* be serializable,so you will need to deal specially with them in saveState() and
restoreState() methods.The simplest thing is to just not save them,and do a new operation again in restoreState(). Sorry but I don't understand. Why won't the normal approach work?
public class SomeComponent { private Log log = LogFactory.getLog(SomeComponent.class); ... }
AIUI, the log object won't be saved in the saveState method, but it will be recreated nicely during the RestoreView phase when a new instance is created for the state to be restored into.
Along the same lines, if your class implements Serializable, you willneed to mark the instance variable transient.I've started using the
following pattern in my Serializable classes, which would work insidea StateHolder as well: private transient Log log = null;
private Log log() {if (log == null) {log = LogFactory.getLog(...);}
return log;} and a typical call looks like: if (log().isDebugEnabled()) {
log().debug(...);} Ok, transient is needed here. But apart from that why won't the standard approach work?
public class SomeThing implements Serializable { private transient Log log = LogFactory.getLog (SomeThing.class); ... if (
log.isDebugEnabled()) { log.debug(...); } } Doesn't the log object get recreated when the deserialization occurs?
No, AIUI.When an object is deserialized, it does *not* execute the variable initializer expressions.Since it was declared transient, there's no state for that object to be restored.
The log() method is quite a lot of boilerplate, and also imposes an additional method call for every isDebugEnabled() call. [The extra call inside the guard is not really relevant as output *is* going to occur at
this point which greatly outweighs the price of the method call]. You can reduce that by one call by protecting only the conditional _expression_, because if you get inside you know there's an object ... but
that means you are relying on the implicit contract between the log instance variable and the log() method.Cheers, Simon
Craig --http://www.irian.atYour JSF powerhouse -JSF Consulting, Development andCourses in English and German
Professional Support for Apache MyFaces
Re: logging (was Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java)
Hmm... didn't we settle on JDK 1.4 a while ago?
Simon has some other arguments on not using JDK logging, see above.
regards,
Martin
On 2/20/06, Craig McClanahan [EMAIL PROTECTED] wrote:
On 2/19/06, Martin Marinschek [EMAIL PROTECTED] wrote:
Indeed.
this really means I'd be willing to switch to JDK logging. What was
the reason again we couldn't do that?
If we are willing to live with a JDK 1.4 or later restriction, no reason
at all. That, however, would seem to be an issue for some current users.
regards,
Martin
Craig
On 2/20/06, Adam Winer [EMAIL PROTECTED] wrote:
Weee if you implement StateHolder, this isn't an issue.
The public no-arg constructor will be used, variable initializer
expressions will run, etc.
If you implement Serializable instead, then Craig's totally right -
transient variables will not be re-initialized. You can deal with
this by adding the log() method, or by providing an implementation
of:
private void readObject(ObjectInputStream in)
... which can re-initialize any transient values.
I am *so* thankful that java.util.logging doesn't force any of
this pain on its users.
-- Adam
On 2/19/06, Craig McClanahan [EMAIL PROTECTED] wrote:
On 2/19/06, Simon Kitching [EMAIL PROTECTED] wrote:
On Sun, 2006-02-19 at 17:56 -0800, Craig McClanahan wrote:
Simon,
Could you do me a favor and publicize this in the Struts community
as
well? The framework code there is littered with static log
instances
to.
Will do.
You might also want to add some notes related to using Log
instances
in Serializable classes (see below).
Will do that also.
MyFaces folks,
There *is* a JSF-specific consideration to think about, if you
have
classes that implement StateHolder (like a UIComponent
implementation). Log instances will generally *not* be
serializable,
so you will need to deal specially with them in saveState() and
restoreState() methods. The simplest thing is to just not save
them,
and do a new operation again in restoreState().
Sorry but I don't understand. Why won't the normal approach work?
public class SomeComponent {
private Log log = LogFactory.getLog(SomeComponent.class);
...
}
AIUI, the log object won't be saved in the saveState method, but it
will
be recreated nicely during the RestoreView phase when a new instance
is
created for the state to be restored into.
Along the same lines, if your class implements Serializable, you
will
need to mark the instance variable transient. I've started using
the
following pattern in my Serializable classes, which would work
inside
a StateHolder as well:
private transient Log log = null;
private Log log() {
if (log == null) {
log = LogFactory.getLog(...);
}
return log;
}
and a typical call looks like:
if (log().isDebugEnabled()) {
log().debug(...);
}
Ok, transient is needed here. But apart from that why won't the
standard
approach work?
public class SomeThing implements Serializable {
private transient Log log = LogFactory.getLog (SomeThing.class);
...
if ( log.isDebugEnabled()) {
log.debug(...);
}
}
Doesn't the log object get recreated when the deserialization
occurs?
No, AIUI. When an object is deserialized, it does *not* execute the
variable initializer expressions. Since it was declared transient,
there's
no state for that object to be restored.
The log() method is quite a lot of boilerplate, and also imposes an
additional method call for every isDebugEnabled() call. [The extra
call
inside the guard is not really relevant as output *is* going to
occur at
this point which greatly outweighs the price of the method call].
You can reduce that by one call by protecting only the conditional
expression, because if you get inside you know there's an object ...
but
that means you are relying on the implicit contract between the log
instance
variable and the log() method.
Cheers,
Simon
Craig
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: svn commit: r378805 - /myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
On Sun, 2006-02-19 at 00:46 +, [EMAIL PROTECTED] wrote:
Author: mmarinschek
Date: Sat Feb 18 16:46:18 2006
New Revision: 378805
URL: http://svn.apache.org/viewcvs?rev=378805view=rev
Log:
minor changes in application-factory, fixed readOnly referral in
HtmlRendererUtils
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Modified:
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
URL:
http://svn.apache.org/viewcvs/myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java?rev=378805r1=378804r2=378805view=diff
==
---
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
(original)
+++
myfaces/core/trunk/impl/src/main/java/org/apache/myfaces/application/ApplicationFactoryImpl.java
Sat Feb 18 16:46:18 2006
@@ -29,12 +29,14 @@
public class ApplicationFactoryImpl
extends ApplicationFactory
{
-private static final Log log =
LogFactory.getLog(ApplicationFactory.class);
+private static final Log log =
LogFactory.getLog(ApplicationFactoryImpl.class);
Just a warning to all developers: when using commons-logging in a
library, STATIC fields must **NOT** be used to store Log objects.
The problem is that the class may be called with various thread-context
classloaders (TCCL) set. However the class is only ever loaded once.
If the Log object is static, then it is initialised using the TCCL of
the very first webapp to use it (ie which forces the class to be loaded)
- including picking up config info from that TCCL. All other webapps
will then use the same Log object - which is not healthy.
Yes this is a nuisance, but it's absolutely necessary to avoid storing
Log instances in static fields for libraries like MyFaces.
Regards,
Simon
