[GitHub] [logging-log4j2] JCgH4164838Gh792C124B5 commented on a change in pull request #271: Proposed fix for LOG4J2-2610 rollover issue:

[GitBox <gi...@apache.org>]

JCgH4164838Gh792C124B5 commented on a change in pull request #271: Proposed fix for LOG4J2-2610 rollover issue:
URL: https://github.com/apache/logging-log4j2/pull/271#discussion_r286277223
 
 

 ##########
 File path: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java
 ##########
 @@ -344,7 +344,12 @@ public synchronized void rollover() {
     }
 
     protected void createFileAfterRollover() throws IOException  {
-        setOutputStream(createOutputStream());
+        try {
 
 Review comment:
   The code path called by line 352, `setOutputStream(createOutputStream(initialTime))`, does attempt to set the creationTime attribute explicitly.  It uses the value of initialTime (which was set previously in `rollover()`).
   
   The 1ms sleep on line 348 was introduced as a mitigation for `RollingAppenderSizeWithTimeTest.testAppender()` failing on Windows during its rapid rollovers (confirmed to happen before any changes in this PR were applied).  A 0ms sleep was tried but that test still failed (it's possible the 1ms sleep succeeds due to a different priority system thread associated with JVM I/O needing to execute).
   
   Does that make sense, or did I misinterpret your question ?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services