logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bruce Brouwer <bruce.brou...@gmail.com>
Subject Re: Broken build
Date Mon, 05 May 2014 02:04:01 GMT
So, who gets to implement it. Ralph, is your time better spent on other
log4j issues, or should I look for new opportunities to help here? Any
suggestions?


On Sun, May 4, 2014 at 9:25 PM, Ralph Goers <ralph.goers@dslextreme.com>wrote:

> It sounds like we are on a similar approach.
>
> Even with registering the Configuration with the StatusFileListener it is
> possible that multiple configurations would want to write to the same file,
> and so should share the same Listener.  In that case closing the file would
> only happen when the last Configuration is unregistered.
>
> Ralph
>
> On May 4, 2014, at 6:06 PM, Bruce Brouwer <bruce.brouwer@gmail.com> wrote:
>
> I was thinking of something along these lines (since Matt was asking me
> for my ideas):
>
> First off, make StatusConsoleListener a base class for
> StatusSystemOutListener and StatusSystemErrListener (maybe inner classes).
> These would no longer need to hold a reference to System.out or System.err,
> so they will implictly follow any changes to System.out or System.err.
>
> Then make a new StatusFileListener for handling files and this class would
> know to close output streams. This class could hold a static map of
> listeners. We could either do reference counting here or my preference -
> register the Configuration with StatusFileListener to get the listener. e.g.
>
> static StatusFileListener register(Configuration config, String fileName)
> { ... }
> static void StatusFileListener release(Configuration config) { ... }
>
> In the XMLConfiguration (and JSONConfiguration) stop and reconfigure
> methods, it would deal with releasing the config. This is hidden behind
> StatusConfiguration, so it might involve XMLConfiguration and
> JSONConfiguration maintaining a reference to StatusConfiguration. I haven't
> totally decided yet what would be best.
>
> (I was in the middle of typing this when I got Ralph's response)
>
>
> On Sun, May 4, 2014 at 9:03 PM, Ralph Goers <ralph.goers@dslextreme.com>wrote:
>
>> I plan on removing the “extends Closeable” from the StatusListener
>> interface and then creating a StatusCloseableListener that extends
>> StatusConsoleListener. The close method will move there. Then in
>> StatusConfiguration we will create the appropriate type based on what the
>> destination is set to.  I may choose to create a static map in
>> StatusCloseableListener and keep a counter for each destination name or I
>> may just use the property that says whether it is a web container and not
>> close the files if it is.  I am not sure yet which would be the best option.
>>
>> Ralph
>>
>> On May 4, 2014, at 4:30 PM, Matt Sicker <boards@gmail.com> wrote:
>>
>> Could you summarize how you'd implement this? Idea sharing could help
>> reduce the overall effort required to elegantly fix this.
>>
>>
>> On 4 May 2014 16:00, Bruce Brouwer <bruce.brouwer@gmail.com> wrote:
>>
>>> I was hoping to hear from Ralph as it sounds like he already started
>>> something.
>>> On May 4, 2014 3:22 PM, "Matt Sicker" <boards@gmail.com> wrote:
>>>
>>>> If you've got a good idea on how to do it, sure.
>>>>
>>>>
>>>> On 4 May 2014 14:04, Bruce Brouwer <bruce.brouwer@gmail.com> wrote:
>>>>
>>>>> I haven't spent much time on this since my initial attempt on 609.
>>>>> Shall I leave it to Ralph to come up with the final solution, or would
you
>>>>> like me to try?
>>>>>  On May 4, 2014 2:35 PM, "Matt Sicker" <boards@gmail.com> wrote:
>>>>>
>>>>>> Looks like there's nothing to synchronise on actually. Guess you
can
>>>>>> just cache them before the check in general.
>>>>>>
>>>>>>
>>>>>> On 4 May 2014 13:25, Matt Sicker <boards@gmail.com> wrote:
>>>>>>
>>>>>>> Oh phew. Well, I'll leave that to you if you wanted to continue
what
>>>>>>> you were working on. All I added was a check on close() to compare
against
>>>>>>> the current System.out and System.err. I'll take a look into
OpenJDK to see
>>>>>>> how to properly lock those (if possible) to prevent fun race
conditions.
>>>>>>>
>>>>>>>
>>>>>>> On 4 May 2014 13:11, Ralph Goers <rgoers@apache.org> wrote:
>>>>>>>
>>>>>>>> No, it can be simpler than that.
>>>>>>>>
>>>>>>>> Sent from my iPad
>>>>>>>>
>>>>>>>> On May 4, 2014, at 10:55 AM, Matt Sicker <boards@gmail.com>
wrote:
>>>>>>>>
>>>>>>>> This is starting to sound like we need a full-blown
>>>>>>>> factory/context/logger implementation of StatusLogger.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4 May 2014 12:46, Ralph Goers <rgoers@apache.org>
wrote:
>>>>>>>>
>>>>>>>>> Also, that doesn't solve the case Remko mentioned of
multiple web
>>>>>>>>> apps writing to a single file.
>>>>>>>>>
>>>>>>>>> Ralph
>>>>>>>>>
>>>>>>>>> On May 4, 2014, at 9:53 AM, Matt Sicker <boards@gmail.com>
wrote:
>>>>>>>>>
>>>>>>>>> So how about adding a check at construction checking
against
>>>>>>>>> System.out and System.err? Really, once you start messing
with those
>>>>>>>>> streams, you can't be sure they'll ever be closed until
the JVM stops or
>>>>>>>>> you manually close it.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 4 May 2014 09:36, Ralph Goers <rgoers@apache.org>
wrote:
>>>>>>>>>
>>>>>>>>>> I see two choices here - maintain a use count or
just let the OS
>>>>>>>>>> close the files.
>>>>>>>>>>
>>>>>>>>>> The second would be pretty easy to do once we move
the web stuff
>>>>>>>>>> to its own module as it can add a property that the
console Appender would
>>>>>>>>>> look for.
>>>>>>>>>>
>>>>>>>>>> The first option is probably better if it could be
made to work
>>>>>>>>>> properly.
>>>>>>>>>>
>>>>>>>>>> Ralph
>>>>>>>>>>
>>>>>>>>>> On May 4, 2014, at 6:38 AM, Bruce Brouwer <
>>>>>>>>>> bruce.brouwer@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> This is what I was starting to investigate with LOG4J2-609.
>>>>>>>>>>
>>>>>>>>>> I don't think this is quite there yet. For one, in
>>>>>>>>>> StatusConsoleListener.close(), System.out and System.err
can change over
>>>>>>>>>> time, so doing the != check might still close something
that at one time
>>>>>>>>>> was System.out but no longer is.
>>>>>>>>>>
>>>>>>>>>> Also, a StatusConsoleListener is shared among different
>>>>>>>>>> JSONConfiguration and XMLConfiguration instances
(think about multiple WARs
>>>>>>>>>> in a Tomcat instance where log4j is in Tomcat's shared
lib directory). If
>>>>>>>>>> we undeployed one of those WARs, it would shutdown
the
>>>>>>>>>> StatusConsoleListener that was shared with the other
WAR deployments.
>>>>>>>>>>
>>>>>>>>>> Also think about where some of these WARs wanted
to use
>>>>>>>>>> System.out and others want to use a log file for
status logging. Because of
>>>>>>>>>> the way these shared loggers are found, only the
first
>>>>>>>>>> StatusConsoleListener registered would actually take
effect. So sometimes
>>>>>>>>>> when you start Tomcat, status logs go to System.out,
other times they go to
>>>>>>>>>> a log file. I'd hate having to debug that one if
I didn't know about this
>>>>>>>>>> issue.
>>>>>>>>>>
>>>>>>>>>> I have an idea of how to address this, but it unfortunately
isn't
>>>>>>>>>> as simple as closing the StatusConsoleListener.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Sat, May 3, 2014 at 10:04 PM, Matt Sicker <boards@gmail.com>wrote:
>>>>>>>>>>
>>>>>>>>>>> Hooray, we've finally figured out the bug. :)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 3 May 2014 19:49, Remko Popma <remko.popma@gmail.com>
wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I just updated from SVN and all tests now
pass.
>>>>>>>>>>>> The build works now. Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Sun, May 4, 2014 at 7:55 AM, Matt Sicker
<boards@gmail.com>wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> I just fixed it in r1592291 haha
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 3 May 2014 17:54, Ralph Goers <ralph.goers@dslextreme.com>wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yes. It cause them to close. Anything
written to System.out
>>>>>>>>>>>>>> or System.err will fail.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On May 3, 2014, at 3:51 PM, Matt
Sicker <boards@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Does closing them do anything?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 3 May 2014 17:10, Ralph Goers
<ralph.goers@dslextreme.com>wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Perhaps we need a StatusFileListerner
when writing to a file?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Ralph
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On May 3, 2014, at 3:03 PM, Ralph
Goers <
>>>>>>>>>>>>>>> ralph.goers@dslextreme.com>
wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> System.out or System.err should
never be closed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Ralph
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On May 3, 2014, at 10:59 AM,
Matt Sicker <boards@gmail.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I've implemented Closeable on
StatusListener in r1592258.
>>>>>>>>>>>>>>> Please try out the unit tests
again and let me know if this solves the
>>>>>>>>>>>>>>> issue on Windows.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 3 May 2014 12:30, Matt Sicker
<boards@gmail.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I think this is actually
a bug. StatusListener should
>>>>>>>>>>>>>>>> implement Closeable, and
when the listeners are cleared, it should loop
>>>>>>>>>>>>>>>> through and close them before
clearing the list of listeners. Otherwise,
>>>>>>>>>>>>>>>> files can stay opened and
Windows still hasn't figured out how to handle
>>>>>>>>>>>>>>>> that.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 3 May 2014 11:22, Remko
Popma <remko.popma@gmail.com>wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks, commenting out
that test to verify my changes was
>>>>>>>>>>>>>>>>> exactly what I was doing
now... :-)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Sun, May 4, 2014 at
1:20 AM, Ralph Goers <
>>>>>>>>>>>>>>>>> ralph.goers@dslextreme.com>
wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Oh, and if you are
trying to do some work just comment
>>>>>>>>>>>>>>>>>> out the @Test of
the failing test - but don’t commit that.
>>>>>>>>>>>>>>>>>> Ralph
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On May 3, 2014, at
9:19 AM, Ralph Goers <
>>>>>>>>>>>>>>>>>> ralph.goers@dslextreme.com>
wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> That happens because
the file is still being referenced
>>>>>>>>>>>>>>>>>> by something when
it is trying to delete it.  It should be because the file
>>>>>>>>>>>>>>>>>> is open but I recall
reading that Windows sometimes holds on to file
>>>>>>>>>>>>>>>>>> references longer
than it should.  This was probably caused by the changes
>>>>>>>>>>>>>>>>>> Matt made to the
unit test framework a month or so ago.  I will bring up my
>>>>>>>>>>>>>>>>>> Windows VM and take
a look at it this afternoon.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Ralph
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On May 3, 2014, at
8:58 AM, Remko Popma <
>>>>>>>>>>>>>>>>>> remko.popma@gmail.com>
wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Yes, windows 7.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Sun, May 4, 2014
at 12:54 AM, Ralph Goers <
>>>>>>>>>>>>>>>>>> ralph.goers@dslextreme.com>
wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> FileOutputTest
was failing for me last week and I
>>>>>>>>>>>>>>>>>>> thought I fixed
it. But it was failing because the file was empty, not
>>>>>>>>>>>>>>>>>>> because it couldn’t
be deleted. I guess you must be running on Windows?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Ralph
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On May 3, 2014,
at 8:44 AM, Remko Popma <
>>>>>>>>>>>>>>>>>>> remko.popma@gmail.com>
wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> > When I run
mvn clean install, I get this problem:
>>>>>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>>>>>> > Failed tests:
>>>>>>>>>>>>>>>>>>> >   FileOutputTest.testConfig
Could not delete
>>>>>>>>>>>>>>>>>>> target\status.log,
last modifed 14/05/04 0:27
>>>>>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>>>>>> > FileOutputTest
has a "CleanFiles" rule that seems to
>>>>>>>>>>>>>>>>>>> fail:
>>>>>>>>>>>>>>>>>>> >     public
RuleChain rules = RuleChain.outerRule(new
>>>>>>>>>>>>>>>>>>> CleanFiles(STATUS_LOG)).around(new
InitialLoggerContext(CONFIG));
>>>>>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>>>>>> > How do I
fix this?
>>>>>>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>>>>>> > Remko
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>>>>>>>>>>> To unsubscribe,
e-mail:
>>>>>>>>>>>>>>>>>>> log4j-dev-unsubscribe@logging.apache.org
>>>>>>>>>>>>>>>>>>> For additional
commands, e-mail:
>>>>>>>>>>>>>>>>>>> log4j-dev-help@logging.apache.org
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>>
>>>>>>>>>> Bruce Brouwer
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <boards@gmail.com>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <boards@gmail.com>
>>>>
>>>
>>
>>
>> --
>> Matt Sicker <boards@gmail.com>
>>
>>
>>
>
>
> --
>
> Bruce Brouwer
>
>
>


-- 

Bruce Brouwer

Mime
View raw message