Bug 56712

Summary: Off-by-one second errors in time calculations in PersistenceManager
Product: Tomcat 7 Reporter: Konstantin Kolinko <knst.kolinko>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: 7.0.54   
Target Milestone: ---   
Hardware: PC   
OS: All   

Description Konstantin Kolinko 2014-07-12 12:22:23 UTC
The fix for bug 56698 (r1608443, r1608448) implemented an automated test for PersistenceManager. Reviewing the manager code to investigate the test failures, I see two issues:

Checks that evaluate idle time of a session in PersistenceManagerBase typically do the following:

> int timeIdle = (int) (session.getIdleTime() / 1000L);
> if (timeIdle > maxIdleBackup) { ... }

I see two errors in those two lines:

Error 1.
The integer division performs truncation. Comparing the code with documentation, the condition there shall be

> if (timeIdle >= maxIdleBackup) { ... }

It does no matter much for a real-world configuration where the times are expected to be tens of seconds, but it matters for the test case that wants to run fast.

Error 2.
StandardSession.getIdleTime() performs a validity check and can throw an ISE which is unexpected here. It shall be replaced with a call to session.getIdleTimeInternal().
Comment 1 Konstantin Kolinko 2014-07-12 12:27:53 UTC
s/ getLastAccessedTime() / getLastAccessedTimeInternal() /
Comment 2 Konstantin Kolinko 2014-07-12 13:59:08 UTC
Fixed in trunk by r1609920. It will be in 8.0.11 onwards.
Comment 3 Konstantin Kolinko 2014-07-12 14:24:23 UTC
Fixed in Tomcat 7 by r1609924 and will be in 7.0.55 onwards.