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> >
