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 Elias Ross <er...@m-qube.com> on 2005/10/31 21:05:37 UTC

JBoss and deadlock issues

For your information:  Log4j has the potential to create deadlocks at
the message rendering step.  The JBoss team has been working on fixing
this issue.

http://jira.jboss.com/jira/browse/JBAS-2393

Scott Stark is trying two approaches.  One is to fix the synchronization
code used in log4j 1.2.12 in JBoss through a patch, which may break user
appenders.  The other is to create log4j-style appenders and DOM
configuration support for the JDK logging framework.  You can read more
in the forum thread linked from the issue.

(This issue I opened about 2 years ago and hasn't really seen much
attention. http://issues.apache.org/bugzilla/show_bug.cgi?id=24159 )

I don't expect this to be fixed for 1.2, but I would hope it gets
addressed for 1.3 as part of the "must-fix list" for 1.3.

I'm not sure what sort of reasonable fix might be employed for the main
framework, I'm hoping to discuss that here.



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Re: JBoss and deadlock issues

Posted by Paul Smith <ps...@aconex.com>.
On 04/11/2005, at 1:38 PM, Mark Womack wrote:

> The wrapper class would only be in 1.2 and really only used to  
> avoid making everyone take the "big" change in the new version of  
> 1.2.  I am assuming that we will do the right thing in 1.3 to not  
> require the use of the wrapper class.
>

So, that would mean any user who commits to this wrapper feature will  
need to 'undo' it if they want to go to 1.3?  Am I reading that  
correctly?  If this is the case, I wonder if it's worth the effort  
(we are spending a lot of time in the maintenance of 1.2.x and not  
getting a whole lot closer to 1.3 where they could take advantage of  
this feature, not that I'm helping much at this point in time!  :()

Paul

>
> And I did find that the conversation on jboss went beyond the  
> 21st.  It is an interesting read I highly recommend.

I'm going to try and read that.  Thanks for the tip.


Paul

Re: JBoss and deadlock issues

Posted by Elias Ross <er...@m-qube.com>.
On Fri, 2005-11-04 at 09:19 -0800, Mark Womack wrote:
> 
> 
> I would much rather get on with 1.3 and do it the right way there.  

I'd rather we think of something better for 1.3 than waste time spinning
on this problem for 1.2.

You may be able to change every class in log4j to have more fine grained
locking semantics, but then you've possibly broken many classes that
have sub classed the appender.  For 1.3, it may be more prudent to add a
separate series of "concurrent" appenders.

Anyway, to further the discussion, I have examples of what a fine-locked
class may look like.  (These are newer than what's in Bugzilla.)

Re: JBoss and deadlock issues

Posted by Mark Womack <mw...@apache.org>.
Well, I was thinking about something we could do quickly for 1.2 without
messing around with the existing code at all. All the stuff I talked about
would be in the wrapper class itself and would not require any changes in
the configurator or appender classes. Maybe changes are required in other
places too. You could choose to use the wrapper or not. No one wants to make
a change like this in the 13th version of the 1.2 release that will affect
the log4j user population as a whole. But as I said, I hate leaving it out
there hanging. I think that we have not handled this one well, for whatever
reasons.

I would much rather get on with 1.3 and do it the right way there.

-Mark

On 11/4/05, Endre Stølsvik <En...@stolsvik.com> wrote:
>
> On Thu, 3 Nov 2005, Mark Womack wrote:
>
> | The wrapper class would only be in 1.2 and really only used to avoid
> making
> | everyone take the "big" change in the new version of 1.2. I am assuming
> that
> | we will do the right thing in 1.3 to not require the use of the wrapper
> | class.
> |
> | The wrapper class would need to have a special (and potentially clumsy)
> | syntax for configuring the wrapper class. Something like:
> |
> | <parameter key="property" value="(key)(value)"/>
> |
> | where key and value would be the property name and value to set. The
> | setProperty method in the wrapper class would parse and set the
> | property/value in the wrapped class. Unless there is something we can do
> | more with bytecode manipulation.
>
> Couldn't one "just" make a special-case for the wrapper? So that if the
> configuration parts of log4j saw the wrapper class specified, if would
> -not- set the properties on the wrapper, but on the wrappee? (except for
> maybe special ones, e.g. starting with "Wrapper:" for key..)
>
> This brings me to the next point: what about "just" introducing a new
> Appender API (a new interface, that is)? In this way, the log4j core
> (configuration system, basically) could "sense" whether it was an old
> Appender (simply instanceof), and thus automatically wrap it, and set the
> properties in the way mentioned above (on the wrappee), or if it was the
> new Appender2, it would configure and use it natively?
>
> The old threads on this list about the subject are also interesting. For
> my part, the coarse-level synching seems the most worrisome, and Elias
> suggestions here for a way more fine grained synching sounds very
> interesting. Contended synchs (which you'd have -often- given the way I've
> understood log4j actually works) are rather expensive, and something you'd
> get a lot in a very multi-threaded application as a webapplication is.
>
> Just my 2 øre.
>
> Regards,
> Endre.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>

Re: JBoss and deadlock issues

Posted by Endre Stølsvik <En...@Stolsvik.com>.
On Thu, 3 Nov 2005, Mark Womack wrote:

| The wrapper class would only be in 1.2 and really only used to avoid making
| everyone take the "big" change in the new version of 1.2. I am assuming that
| we will do the right thing in 1.3 to not require the use of the wrapper
| class.
| 
| The wrapper class would need to have a special (and potentially clumsy)
| syntax for configuring the wrapper class. Something like:
| 
| <parameter key="property" value="(key)(value)"/>
| 
| where key and value would be the property name and value to set. The
| setProperty method in the wrapper class would parse and set the
| property/value in the wrapped class. Unless there is something we can do
| more with bytecode manipulation.

Couldn't one "just" make a special-case for the wrapper? So that if the 
configuration parts of log4j saw the wrapper class specified, if would 
-not- set the properties on the wrapper, but on the wrappee? (except for 
maybe special ones, e.g. starting with "Wrapper:" for key..)

This brings me to the next point: what about "just" introducing a new 
Appender API (a new interface, that is)? In this way, the log4j core 
(configuration system, basically) could "sense" whether it was an old 
Appender (simply instanceof), and thus automatically wrap it, and set the 
properties in the way mentioned above (on the wrappee), or if it was the 
new Appender2, it would configure and use it natively?

The old threads on this list about the subject are also interesting. For 
my part, the coarse-level synching seems the most worrisome, and Elias 
suggestions here for a way more fine grained synching sounds very 
interesting. Contended synchs (which you'd have -often- given the way I've 
understood log4j actually works) are rather expensive, and something you'd 
get a lot in a very multi-threaded application as a webapplication is.

Just my 2 �re.

Regards,
Endre.

Re: JBoss and deadlock issues

Posted by Mark Womack <mw...@apache.org>.
The wrapper class would only be in 1.2 and really only used to avoid making
everyone take the "big" change in the new version of 1.2. I am assuming that
we will do the right thing in 1.3 to not require the use of the wrapper
class.

The wrapper class would need to have a special (and potentially clumsy)
syntax for configuring the wrapper class. Something like:

<parameter key="property" value="(key)(value)"/>

where key and value would be the property name and value to set. The
setProperty method in the wrapper class would parse and set the
property/value in the wrapped class. Unless there is something we can do
more with bytecode manipulation.

And I did find that the conversation on jboss went beyond the 21st. It is an
interesting read I highly recommend.

-Mark

On 11/3/05, Paul Smith <ps...@aconex.com> wrote:
>
> that's very intriguing on face value. Bit of maintenance effort? If this
> is for 1.2, are you saying that a 1.3 upgrade by our users would be
> required to 'drop' their use of the wrapper classes in favour of something
> else, or do you see these wrapper impl's staying supported features longer
> term?
> I'm a bit confused by the "....wrapper class will not have getter/setters
> for the class being wrapped"... How does the configuration of the 'wrapped'
> class get configured?
>
> Paul
>
> On 04/11/2005, at 1:09 PM, Mark Womack wrote:
>
> OK, stop me if this sounds completely crazy. It might.
>
> What if we created a wrapper class for subclasses of AppenderSkeleton that
> implemented Elias's version of doAppend with the changed synchronization
> logic? That wrapper class would be part of the 1.2 release but would not
> be used in the current classes, meaning that the current code would be in
> place by default, not messing anyone up right away.
>
> But, if desired, the user could use the new wrapper class to wrap the
> existing subclasses of AppenderSkeleton, and they could control it all from
> the configuration file. They would specify the wrapper class for the
> appender class, parameters would specify the class they want to wrap and the
> specific parameters to set. The wrapper class would just do call thrus to
> the class being wrapped, except for the doAppend call, which would be new
> behavior. Some care would need to taken to support the property settings
> since the wrapper class will not have getter/setters for the class being
> wrapped.
>
> Does that sound too wacky? It would preserve the existing log4j 1.2 api,
> but would give users some recourse to correct the locking problem if they
> are seeing it. I need to look at more code, but thought I would bounce it
> out there.
>
> -Mark
>
> On 11/3/05, Mark Womack <mw...@apache.org> wrote:
> >
> > Yes, it is on the "list" for 1.3.
> >
> > The last message in the thread is from 10/21. Do you have any update
> > since then? I really hate leaving this hanging in 1.2. I would like to
> > find a solution that does not break subclasses of AppenderSkeleton. Don't
> > know what that would be yet.
> >
> > -Mark
> >
> > On 10/31/05, Elias Ross < eross@m-qube.com> wrote:
> > >
> > >
> > > For your information: Log4j has the potential to create deadlocks at
> > > the message rendering step. The JBoss team has been working on fixing
> > > this issue.
> > >
> > > http://jira.jboss.com/jira/browse/JBAS-2393<http://www.google.com/url?sa=D&q=http%3A%2F%2Fjira.jboss.com%2Fjira%2Fbrowse%2FJBAS-2393>
> > >
> > > Scott Stark is trying two approaches. One is to fix the
> > > synchronization
> > > code used in log4j 1.2.12 in JBoss through a patch, which may break
> > > user
> > > appenders. The other is to create log4j-style appenders and DOM
> > > configuration support for the JDK logging framework. You can read more
> > > in the forum thread linked from the issue.
> > >
> > > (This issue I opened about 2 years ago and hasn't really seen much
> > > attention. http://issues.apache.org/bugzilla/show_bug.cgi?id=24159<http://www.google.com/url?sa=D&q=http%3A%2F%2Fissues.apache.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D24159>)
> > >
> > > I don't expect this to be fixed for 1.2, but I would hope it gets
> > > addressed for 1.3 as part of the "must-fix list" for 1.3.
> > >
> > > I'm not sure what sort of reasonable fix might be employed for the
> > > main
> > > framework, I'm hoping to discuss that here.
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > > For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > >
> > >
> >
>
>
>

Re: JBoss and deadlock issues

Posted by Paul Smith <ps...@aconex.com>.
that's very intriguing on face value.  Bit of maintenance effort?  If  
this is for 1.2, are you saying that a 1.3 upgrade by our users would  
be required to 'drop' their use of the wrapper classes in favour of  
something else, or do you see these wrapper impl's staying supported  
features longer term?

I'm a bit confused by the "....wrapper class will not have getter/ 
setters for the class being wrapped"... How does the configuration of  
the 'wrapped' class get configured?

Paul

On 04/11/2005, at 1:09 PM, Mark Womack wrote:

> OK, stop me if this sounds completely crazy.  It might.
>
> What if we created a wrapper class for subclasses of  
> AppenderSkeleton that implemented Elias's version of doAppend with  
> the changed synchronization logic?  That wrapper class would be  
> part of the 1.2 release but would not be used in the current  
> classes, meaning that the current code would be in place by  
> default, not messing anyone up right away.
>
> But, if desired, the user could use the new wrapper class to wrap  
> the existing subclasses of AppenderSkeleton, and they could control  
> it all from the configuration file.  They would specify the wrapper  
> class for the appender class, parameters would specify the class  
> they want to wrap and the specific parameters to set.  The wrapper  
> class would just do call thrus to the class being wrapped, except  
> for the doAppend call, which would be new behavior.  Some care  
> would need to taken to support the property settings since the  
> wrapper class will not have getter/setters for the class being  
> wrapped.
>
> Does that sound too wacky?  It would preserve the existing log4j  
> 1.2 api, but would give users some recourse to correct the locking  
> problem if they are seeing it.  I need to look at more code, but  
> thought I would bounce it out there.
>
> -Mark
>
> On 11/3/05, Mark Womack <mw...@apache.org> wrote:
> Yes, it is on the "list" for 1.3.
>
> The last message in the thread is from 10/21.  Do you have any  
> update since then?  I really hate leaving this hanging in 1.2.  I  
> would like to find a solution that does not break subclasses of  
> AppenderSkeleton.  Don't know what that would be yet.
>
> -Mark
>
>
> On 10/31/05, Elias Ross < eross@m-qube.com> wrote:
>
> For your information:  Log4j has the potential to create deadlocks at
> the message rendering step.  The JBoss team has been working on fixing
> this issue.
>
> http://jira.jboss.com/jira/browse/JBAS-2393
>
> Scott Stark is trying two approaches.  One is to fix the  
> synchronization
> code used in log4j 1.2.12 in JBoss through a patch, which may break  
> user
> appenders.  The other is to create log4j-style appenders and DOM
> configuration support for the JDK logging framework.  You can read  
> more
> in the forum thread linked from the issue.
>
> (This issue I opened about 2 years ago and hasn't really seen much
> attention. http://issues.apache.org/bugzilla/show_bug.cgi?id=24159 )
>
> I don't expect this to be fixed for 1.2, but I would hope it gets
> addressed for 1.3 as part of the "must-fix list" for 1.3.
>
> I'm not sure what sort of reasonable fix might be employed for the  
> main
> framework, I'm hoping to discuss that here.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>


Re: JBoss and deadlock issues

Posted by Mark Womack <mw...@apache.org>.
OK, stop me if this sounds completely crazy. It might.

What if we created a wrapper class for subclasses of AppenderSkeleton that
implemented Elias's version of doAppend with the changed synchronization
logic? That wrapper class would be part of the 1.2 release but would not be
used in the current classes, meaning that the current code would be in place
by default, not messing anyone up right away.

But, if desired, the user could use the new wrapper class to wrap the
existing subclasses of AppenderSkeleton, and they could control it all from
the configuration file. They would specify the wrapper class for the
appender class, parameters would specify the class they want to wrap and the
specific parameters to set. The wrapper class would just do call thrus to
the class being wrapped, except for the doAppend call, which would be new
behavior. Some care would need to taken to support the property settings
since the wrapper class will not have getter/setters for the class being
wrapped.

Does that sound too wacky? It would preserve the existing log4j 1.2 api, but
would give users some recourse to correct the locking problem if they are
seeing it. I need to look at more code, but thought I would bounce it out
there.

-Mark

On 11/3/05, Mark Womack <mw...@apache.org> wrote:
>
> Yes, it is on the "list" for 1.3.
>
> The last message in the thread is from 10/21. Do you have any update since
> then? I really hate leaving this hanging in 1.2. I would like to find a
> solution that does not break subclasses of AppenderSkeleton. Don't know what
> that would be yet.
>
> -Mark
>
> On 10/31/05, Elias Ross <er...@m-qube.com> wrote:
> >
> >
> > For your information: Log4j has the potential to create deadlocks at
> > the message rendering step. The JBoss team has been working on fixing
> > this issue.
> >
> > http://jira.jboss.com/jira/browse/JBAS-2393<http://www.google.com/url?sa=D&q=http%3A%2F%2Fjira.jboss.com%2Fjira%2Fbrowse%2FJBAS-2393>
> >
> > Scott Stark is trying two approaches. One is to fix the synchronization
> > code used in log4j 1.2.12 in JBoss through a patch, which may break user
> > appenders. The other is to create log4j-style appenders and DOM
> > configuration support for the JDK logging framework. You can read more
> > in the forum thread linked from the issue.
> >
> > (This issue I opened about 2 years ago and hasn't really seen much
> > attention. http://issues.apache.org/bugzilla/show_bug.cgi?id=24159<http://www.google.com/url?sa=D&q=http%3A%2F%2Fissues.apache.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D24159>)
> >
> > I don't expect this to be fixed for 1.2, but I would hope it gets
> > addressed for 1.3 as part of the "must-fix list" for 1.3.
> >
> > I'm not sure what sort of reasonable fix might be employed for the main
> > framework, I'm hoping to discuss that here.
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > For additional commands, e-mail: log4j-dev-help@logging.apache.org
> >
> >
>

Re: JBoss and deadlock issues

Posted by Mark Womack <mw...@apache.org>.
Yes, it is on the "list" for 1.3.

The last message in the thread is from 10/21. Do you have any update since
then? I really hate leaving this hanging in 1.2. I would like to find a
solution that does not break subclasses of AppenderSkeleton. Don't know what
that would be yet.

-Mark

On 10/31/05, Elias Ross <er...@m-qube.com> wrote:
>
>
> For your information: Log4j has the potential to create deadlocks at
> the message rendering step. The JBoss team has been working on fixing
> this issue.
>
> http://jira.jboss.com/jira/browse/JBAS-2393
>
> Scott Stark is trying two approaches. One is to fix the synchronization
> code used in log4j 1.2.12 in JBoss through a patch, which may break user
> appenders. The other is to create log4j-style appenders and DOM
> configuration support for the JDK logging framework. You can read more
> in the forum thread linked from the issue.
>
> (This issue I opened about 2 years ago and hasn't really seen much
> attention. http://issues.apache.org/bugzilla/show_bug.cgi?id=24159 )
>
> I don't expect this to be fixed for 1.2, but I would hope it gets
> addressed for 1.3 as part of the "must-fix list" for 1.3.
>
> I'm not sure what sort of reasonable fix might be employed for the main
> framework, I'm hoping to discuss that here.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>