Contrib.social

Rounding mistake in getRequestTime()

Open on Drupal.org →
Created on 14 March 2025, 27 days ago

Problem/Motivation

TestTime->getRequestTime() can produce inconsistent results due to rounding in the calculation.

Steps to reproduce

We did the following in a kernel test:

  public function testTimeRoundingError(): void {
    \Drupal::time()->setTime('+100 seconds');
    $timestamp = \Drupal::time()->getRequestTime();
    for ($i = 1; $i <= 10; ++$i) {
      time_nanosleep(0, 100000000);
      $this->assertSame(0, $timestamp - \Drupal::time()->getRequestTime(), "after 0.$i seconds");
    }
  }

This fails for us with a random-ish value in the message, e.g. "after 0.3 seconds" then "after 0.7 seconds" etc.

I looked into the calculation, and it seems way too smart.

Proposed resolution

Add the test.
Fix the calculation.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

1.0

Component

Miscellaneous

Created by

🇩🇪Germany donquixote

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @donquixote
  • Comment 27 days ago →
    🇩🇪Germany donquixote

    Some equation juggling which may or may not be helpful.
    Note that `(int)` behaves like floor() for positive numbers and like ceil() for negative numbers, which makes it harder to reason about.
    But the bug still happens if we replace (int) with floor().

    getRequestTime() = getCurrentTime() - (REAL.CURRENT_TIME - REAL.REQUEST_TIME)
    getCurrentTime() = floor(getCurrentMicroTime())
    getCurrentMicroTime() = SPECIFIED_TIME + getMicroTimePassed()
    getMicroTimePassed() = REAL.CURRENT_MICRO_TIME - TIME_STARTED

    getRequestTime() = floor(REAL.CURRENT_MICRO_TIME - TIME_STARTED)
    - REAL.CURRENT_TIME
    + REAL.REQUEST_TIME
    + SPECIFIED_TIME

    getRequestTime() = floor(REAL.CURRENT_TIME + REAL.CURRENT_TIME_FRACTION - TIME_STARTED)
    - REAL.CURRENT_TIME
    + REAL.REQUEST_TIME
    + SPECIFIED_TIME

    getRequestTime() = floor(REAL.CURRENT_TIME_FRACTION - TIME_STARTED)
    + REAL.REQUEST_TIME
    + SPECIFIED_TIME

    getRequestTime() = floor(REAL.CURRENT_TIME_FRACTION - TIME_STARTED_FRACTION)
    - TIME_STARTED_SECONDS
    + REAL.REQUEST_TIME
    + SPECIFIED_TIME

    Here we split float variables into varname_int + varname_fraction, so that we can move the integer part out of the floor() call.
    This would not be possible with just (int) due to the asymmetry mentioned above.

    The problem would go away if TIME_STARTED_FRACTION is zero.

  • Comment 22 days ago →
    🇯🇵Japan ptmkenny

    Thanks for reporting this. As a maintainer, I'm happy to review an MR that contains a fix for this and a test, but I don't have the time to work on the code myself.

    I also didn't start maintaining this module until a few years after that code was committed, so I'm not able to explain why it is written the way it is.

  • Comment 22 days ago →
    🇯🇵Japan ptmkenny

    Duplicate, please ignore.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024