Problem/Motivation

While doing some unrelated performance testing Blackfire told me that the $more_entropy option of uniqid() should be used because if $more_entropy is omitted or set to false:

  • on Cygwin, the function does not work;
  • on Windows, it yields non-unique values;
  • on other OSes, it calls usleep(1), pausing your entire application for 1 microsecond.
  • Therefore, the only way to make uniqid() generate reasonably unique values is to call it with true as second argument (uniqid('...', true)).

Proposed resolution

Ensure all usages in core use the second argument.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3184769-7.patch3.96 KBrenrhaf
#2 3184769-2.patch4.03 KBalexpott

Issue fork drupal-3184769

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
StatusFileSize
new4.03 KB

This won't result in any huge speed up but it will remove a notice when profiling on blackfire so less noise.

Status: Needs review » Needs work

The last submitted patch, 2: 3184769-2.patch, failed testing. View results

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

renrhaf’s picture

StatusFileSize
new3.96 KB

Rerolled patch for Drupal 9.5.x
+1 RTBC

renrhaf’s picture

Status: Needs work » Needs review

Vighneshh made their first commit to this issue’s fork.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Seems there were some errors errors in the build.

And if this is going to be enforced think it will need a phpstan check with a test case

jungle made their first commit to this issue’s fork.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mfb’s picture

It's definitely a good idea to use the second argument because of the issue of non-unique IDs on some platforms, as well as the fact that all platforms could have non-unique IDs when the system clock changes. But given that uniqid() wastefully sleeps as described in the issue summary, I'd propose closing this issue in favor of #2972100: Remove usage of uniqid.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.