I was hoping to hear from Ralph as it sounds like he already started something. On May 4, 2014 3:22 PM, "Matt Sicker" <boa...@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.brou...@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" <boa...@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 <boa...@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 <rgo...@apache.org> wrote: >>>> >>>>> No, it can be simpler than that. >>>>> >>>>> Sent from my iPad >>>>> >>>>> On May 4, 2014, at 10:55 AM, Matt Sicker <boa...@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 <rgo...@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 <boa...@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 <rgo...@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.brou...@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 <boa...@gmail.com>wrote: >>>>>>> >>>>>>>> Hooray, we've finally figured out the bug. :) >>>>>>>> >>>>>>>> >>>>>>>> On 3 May 2014 19:49, Remko Popma <remko.po...@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 <boa...@gmail.com>wrote: >>>>>>>>> >>>>>>>>>> I just fixed it in r1592291 haha >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 3 May 2014 17:54, Ralph Goers <ralph.go...@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 <boa...@gmail.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Does closing them do anything? >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 3 May 2014 17:10, Ralph Goers <ralph.go...@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.go...@dslextreme.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>> System.out or System.err should never be closed. >>>>>>>>>>>> >>>>>>>>>>>> Ralph >>>>>>>>>>>> >>>>>>>>>>>> On May 3, 2014, at 10:59 AM, Matt Sicker <boa...@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 <boa...@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.po...@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.go...@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.go...@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.po...@gmail.com> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Yes, windows 7. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Sun, May 4, 2014 at 12:54 AM, Ralph Goers < >>>>>>>>>>>>>>> ralph.go...@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.po...@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-unsubscr...@logging.apache.org >>>>>>>>>>>>>>>> For additional commands, e-mail: >>>>>>>>>>>>>>>> log4j-dev-h...@logging.apache.org >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>>>> Matt Sicker <boa...@gmail.com> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Matt Sicker <boa...@gmail.com> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Matt Sicker <boa...@gmail.com> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Matt Sicker <boa...@gmail.com> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Matt Sicker <boa...@gmail.com> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> Bruce Brouwer >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Matt Sicker <boa...@gmail.com> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Matt Sicker <boa...@gmail.com> >>>>> >>>>> >>>> >>>> >>>> -- >>>> Matt Sicker <boa...@gmail.com> >>>> >>> >>> >>> >>> -- >>> Matt Sicker <boa...@gmail.com> >>> >> > > > -- > Matt Sicker <boa...@gmail.com> >
