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.brou...@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" <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> >> > -- Matt Sicker <boa...@gmail.com>
