Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Review System.nanoTime() usages #8532

Closed
sbordet opened this issue Sep 1, 2022 · 0 comments · Fixed by #8535
Closed

Review System.nanoTime() usages #8532

sbordet opened this issue Sep 1, 2022 · 0 comments · Fixed by #8535
Assignees
@sbordet
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Sep 1, 2022

Jetty version(s)
10+

Description
System.nanoTime(), as specified in its javadocs, can roll-over.
Because of that, it's wrong to use the < operator between values, as well as Math.min() etc.

This problem has already been reported in CometD, see cometd/cometd#1215.

The whole codebase should be reviewed so that System.nanoTime() is properly used.
@sbordet sbordet added the Bug For general bugs on Jetty side label Sep 1, 2022
@sbordet sbordet self-assigned this Sep 1, 2022
sbordet added a commit that referenced this issue Sep 2, 2022
@sbordet
Fixes #8532 - Review System.nanoTime() usages.
2809724 
Introduced o.e.j.util.NanoTime class to deal with nanoTimes.

Now NanoTime.now() should be used instead of System.nanoTime(),
and various <unit>elapsed*() methods to calculate elapsed times.

Furthermore, comparing 2 nanoTimes should be done via isBefore(),
rather than using the < operator, which is wrong as specified in
the System.nanoTime() javadocs.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked a pull request Sep 2, 2022 that will close this issue
Fixes #8532 - Review System.nanoTime() usages. #8535
Merged
sbordet added a commit that referenced this issue Sep 6, 2022
@sbordet
Fixes #8532 - Review System.nanoTime() usages. (#8535)
77ad018 
* Fixes #8532 - Review System.nanoTime() usages.

Introduced o.e.j.util.NanoTime class to deal with nanoTimes.

Now NanoTime.now() should be used instead of System.nanoTime(),
and various <unit>[elapsed|since|until]() methods to calculate nanoTimes.

Furthermore, comparing 2 nanoTimes should be done via isBefore(),
rather than using the < operator, which is wrong as specified in
the System.nanoTime() javadocs.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@sbordet sbordet

Labels
Bug For general bugs on Jetty side
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #8532 - Review System.nanoTime() usages. jetty/jetty.project
1 participant
@sbordet