Skip to content

8297531: sun/security/krb5/MicroTime.java fails with "Exception: What? only 100 musec precision?"#23867

Closed
wangweij wants to merge 3 commits intoopenjdk:masterfrom
wangweij:8297531
Closed

8297531: sun/security/krb5/MicroTime.java fails with "Exception: What? only 100 musec precision?"#23867
wangweij wants to merge 3 commits intoopenjdk:masterfrom
wangweij:8297531

Conversation

@wangweij
Copy link
Contributor

@wangweij wangweij commented Mar 3, 2025

Loosen the check; the modified test is sufficient to demonstrate sub-millisecond precision.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8297531: sun/security/krb5/MicroTime.java fails with "Exception: What? only 100 musec precision?" (Bug - P3)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23867/head:pull/23867
$ git checkout pull/23867

Update a local copy of the PR:
$ git checkout pull/23867
$ git pull https://git.openjdk.org/jdk.git pull/23867/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23867

View PR using the GUI difftool:
$ git pr show -t 23867

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23867.diff

Using Webrev

Link to Webrev Comment

8297531: sun/security/krb5/MicroTime.java fails with "Exception: What? only 100 musec precision?"
@bridgekeeper
Copy link

bridgekeeper bot commented Mar 3, 2025

👋 Welcome back weijun! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Mar 3, 2025

@wangweij This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8297531: sun/security/krb5/MicroTime.java fails with "Exception: What? only 100 musec precision?"

Reviewed-by: mullan, abarashev

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 32 new commits pushed to the master branch:

  • 5598792: 8351064: JFR: Consistent timestamps
  • fe806ca: 8350605: assert(!heap->is_uncommit_in_progress()) failed: Cannot uncommit bitmaps while resetting them
  • 5b8d349: 4745837: Make grouping usage during parsing apparent in relevant NumberFormat methods
  • 4aa4b46: 8351154: Use -ftrivial-auto-var-init=pattern for clang too
  • daf0213: 8350924: javax/swing/JMenu/4213634/bug4213634.java fails
  • 6a31aae: 8350594: Misleading warning about install dir for DMG packaging
  • 216f113: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null
  • 3230894: 8348561: Add aarch64 intrinsics for ML-DSA
  • 8073914: 8350974: The os_cpu VM_STRUCTS, VM_TYPES, etc have no declarations and should be removed
  • 7ee89a5: 8350893: Use generated names for hand generated opto runtime blobs
  • ... and 22 more: https://git.openjdk.org/jdk/compare/f47232ad7129e40bdc433525a66de2ca6657f211...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 3, 2025
@openjdk
Copy link

openjdk bot commented Mar 3, 2025

@wangweij The following label will be automatically applied to this pull request:

  • security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the security security-dev@openjdk.org label Mar 3, 2025
@wangweij wangweij closed this Mar 3, 2025
@wangweij wangweij reopened this Mar 3, 2025
@mlbridge
Copy link

mlbridge bot commented Mar 3, 2025

Webrevs

}
}
// We believe a nice KerberosTime can at least tell the
// difference of 100 musec.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's "musec"? Should it be "nanoseconds"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I see that it's actually microsecond.

if (count < 10000) {
// Before this change, KerberosTime was implemented in milliseconds.
// Now there should be more.
if (count < 1001) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 1001 and not 1000?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind that, 1001 is needed to make sure we have a precision at least better than 1 millisecond. So 1001 unique timestamps out of of 1M gives us just that.

Copy link
Member

@artur-oracle artur-oracle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +47 to +49
// Before this change, KerberosTime was implemented in milliseconds.
// Now there should be more.
if (count < 1001) {
Copy link
Member

@seanjmullan seanjmullan Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this comment more specific? What change was this? And what is the precision after the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. I was pretending the comment was written right when the bug was fixed.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Mar 4, 2025
@wangweij
Copy link
Contributor Author

wangweij commented Mar 4, 2025

/integrate

@openjdk
Copy link

openjdk bot commented Mar 4, 2025

Going to push as commit 0753376.
Since your change was applied there have been 32 commits pushed to the master branch:

  • 5598792: 8351064: JFR: Consistent timestamps
  • fe806ca: 8350605: assert(!heap->is_uncommit_in_progress()) failed: Cannot uncommit bitmaps while resetting them
  • 5b8d349: 4745837: Make grouping usage during parsing apparent in relevant NumberFormat methods
  • 4aa4b46: 8351154: Use -ftrivial-auto-var-init=pattern for clang too
  • daf0213: 8350924: javax/swing/JMenu/4213634/bug4213634.java fails
  • 6a31aae: 8350594: Misleading warning about install dir for DMG packaging
  • 216f113: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null
  • 3230894: 8348561: Add aarch64 intrinsics for ML-DSA
  • 8073914: 8350974: The os_cpu VM_STRUCTS, VM_TYPES, etc have no declarations and should be removed
  • 7ee89a5: 8350893: Use generated names for hand generated opto runtime blobs
  • ... and 22 more: https://git.openjdk.org/jdk/compare/f47232ad7129e40bdc433525a66de2ca6657f211...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Mar 4, 2025
@openjdk openjdk bot closed this Mar 4, 2025
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 4, 2025
@openjdk
Copy link

openjdk bot commented Mar 4, 2025

@wangweij Pushed as commit 0753376.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@wangweij wangweij deleted the 8297531 branch March 4, 2025 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrated Pull request has been integrated security security-dev@openjdk.org

Development

Successfully merging this pull request may close these issues.

3 participants