[
https://issues.apache.org/jira/browse/LOG4J2-1653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15794618#comment-15794618
]
Georg Friedrich commented on LOG4J2-1653:
-----------------------------------------
Thanks for doing another change, but there is still something left.
* You could remove the "findIncrement" method in the "CronExpression" class as
it is not used anymore.
* The "shutdown" method leaves all scheduled tasks scheduled, so you will most
probably always run into the first timeout. In worst case a scheduled task
started the execution while awaiting the first termination which again results
in the interruption of this task as you call "shutdownNow" after the timeout. I
don't see the reason why you don't clear the queue before calling "shutdown"
(as in the patch).
* The new "getTimeBefore" method seems to have a slightly different contract
than "getTimeAfter". "getTimeAfter" only returns timestamps that are really
after the given timestamp (even when the timestamp matches the cron
expression). "getTimeBefore" might return the targetDate as the while loop only
uses "after" without checking the equality of both timestamps.
* You should also consider some "abort" case. Just in case something is
horribly wrong. "getTimeAfter" uses a case where year gets above 2999. Maybe
use something like "if year is below 1970 -> abort".
* In "setTriggeringPolicy" "triggeringPolicy.initialize(this)" might still be
called without a successful "compareAndSet". This results in a scheduled
TriggeringPolicy which is not set in the RollingFileManager. You should move
this line into the if-statement.
> CronTriggeringPolicy uses wrong naming and produces NPE
> -------------------------------------------------------
>
> Key: LOG4J2-1653
> URL: https://issues.apache.org/jira/browse/LOG4J2-1653
> Project: Log4j 2
> Issue Type: Bug
> Affects Versions: 2.7
> Reporter: Georg Friedrich
> Assignee: Ralph Goers
> Priority: Critical
> Fix For: 2.8
>
> Attachments: ConfigurationScheduler.patch, CronTriggeringPolicy.patch
>
>
> After having worked on LOG4J2-1649 I found another serious issue in
> combination with the CronTriggeringPolicy.
> The policy has some very weird behaviour when it comes to naming rolled over
> files and also creates a NPE on specific configurations.
> The following is broken:
> * when using "evaluateOnStartup" a NPE is the result of an immediate rollover
> * when no rollover is happing at startup the first rollover produces a file
> that uses the time of the rollover (e.g. rollover is happening at midnight
> 2010-05-05 producing a rolled over file named log-2010-05-05)
> * but it becomes worse: all files after the first rollover are named using a
> date of the "previous rollover date minus a second" - when using the previous
> example this results in:
> ** first rollover happening at midnight 2010-05-05, resulting in file
> log-2010-05-05
> ** next rollover happening at 2010-05-06, resulting in file log-2010-05-04
> ** next rollover happening at 2010-05-07, resulting in file log-2010-05-05
> again (!) so the previously saved file gets removed!
> I would expect the file to be named after the content it contains. E.g. a
> file rolled over at 2010-05-05 should be named log-2010-05-04 as it contains
> all the data of the 2010-05-04.
> So I decided to write a patch for those problems too (again the sources of
> Log4J2 2.7 were used). Unfortunately I needed a method to calculate the last
> cron date. The CronExpression class has such a method ("getTimeBefore") but
> nobody implemented this one since years.
> The only quick solution I found: I used another 3rd party library to fix this
> called cronutils. The solution I wrote uses the latest version of this
> library which now only supports Java 8.
> My guess is that you don't want Log4J2 to only support Java 8 - if this is
> the case you will have to use a different library/version or whatever to be
> able to calculate the last cron date.
> This patch should also fix the following bugs: LOG4J2-1640, LOG4J2-1621,
> LOG4J2-1487, LOG4J2-1474 (and maybe even more - I didn't feel like searching
> the whole Jira ;-) )
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-dev-h...@logging.apache.org
