I get this too, latest master. I did not used to get it a few weeks ago.

I also get this in MOODLE_25_STABLE.

Perhaps author of the test file, Sam Hemelryk, may have some ideas?

I haven't altered any of the standard moodle cache settings.
Tried with zend opcache enabled and disabled in php.ini. Fails under both conditions.

Windows 2008 R2, PHP 5.5.1

I am not seeing these fails currently under Windows. I'm running PHP 5.4.7 with memcached and opcache running.

I also tested and these failures no longer occur for me. Since that's two of us, I think it's probably okay to close the issue now as it probably was fixed by some other change. David/Tim - If it still happens for you on current master, we can reopen?

Well, we've now seen this twice on 64bit Ubuntu virtual machine in the last couple of days.

Random string depends on mt_srand() at this point. Which when called is initialized to the fraction of a second that you called microtime. So on machines with microsecond resolution, there is a 1 in a million chance you will get the same value when calling it twice in the same code.

The php manual at http://www.php.net/manual/en/function.mt-srand.php indicates that the mt_srand call is not longer required since php 4.2. As php initializes it itself. If that's the case, it will only get initialized once per session and would remove the unit testing errors. I've not investigated the implications of how PHP initializes it automatically in the case of mod_php or cli. I don't expect it can be worse than the chances we have of duplicating it.

Given my current understanding of this issue, I'm of the opinion we remove the mt_srand call in random_string and see if we can ever reproduce the error again. I feel I'm likely to see it again as it's happening once every 1-2 days. Are there any security implications I'm not thinking of by making this change?

All input welcome and encouraged.

Interesting. I'm still able to reproduce this fail every single time in my windows environment. (moodle 2.5.2 - PHP 5.5.1) I know that windows PHP random number support historically has been a lot different to nix. (/dev/urandom etc..). I'm willing to speculate that the test case functions differently to core moodle, because otherwise - wouldn't I expect to see strange behaviour from within the application?

Could you please try latest PHP 5.5.5? (There is no reason to use or test with outdated PHP version in Windows.)

This is also related and supports the seeding the same random series hypothesis; http://stackoverflow.com/questions/18889244/php-5-5-on-windows-microtime-behavior

More information: https://bugs.php.net/bug.php?id=64633 indicates on windows you only get 15.6ms resolution for microtime() on php 5.4.14+ and php 5.5.1+

I'm publishing a master branch that I think will resolve this issue.

Please comment and post any objections to using this as a fix.

Have you looked through git history to see if there was any comment when the call to seed the random number generator was added?

Rather that just removing that call, why not either A) move it to setup.php, or B) use a static so that we do it only once each PHP process?

It was added as part of the function addition;

commit 4c1ba3ffc74355f79a6114774fc81bea0a6dcd66
Author: Petr Skoda <skodak@moodle.org>
Date: Tue Nov 17 16:11:25 2009 +0000

MDL-18006 MDL-18807 MDL-20853 merging sam's changes from MOODLE_19_STABLE, going to fix some more problems in this new code soon

I do not have permissions to view any of these tracker items.

The PHP manual indicates that it already does the work for you so there is no need for calling mt_srand at all. This means we don't need to proceed with either of A) or B). If we think it's really required, moving it to setup.php may be useful. I'm not sure how that interacts with unit tests. Either way, we want to initialize the generator once per page rather than once per call.

We don't support PHP4, and the requirement for initialization was removed in 4.2. Is there a reason you think it's good for us to seed the generator? Is it because we don't trust PHP?

I believe there is no reason to use it any more, the linked issues above are not relevant.

+3 to remove this line, thanks a lot for the report and patch

please submit to integration when ready, I guess execution of unit tests is enough as testing instructions

Petr,

I've now tried this on my environment with 5.5.5 and I get the same errors.

Thanks David, could you please also try to delete the following line form your lib/moodlelib.php and run the tests?

mt_srand ((double) microtime() * 1000000);

I've updated the current patch to remove all versions of mt_srand from the code.

I've now looks at all calls to srand. I've successfully removed most of those, however there is one that I have question about. I'd like a 2nd/3rd opinion on. I've added Matt Clarkson as he wrote the code originally.

group/autogroup.php:87   srand($data->seed)

I'm not 100% clear if there is value in having the seed set to a specific time for each load of the form. Does the value get updated each time the form loads? My Moodle form fu is still low. If it does, it's safe to remove. But if not, I suspect there is some value it in but I'm unclear of what. I would have assumed the call would have been srand(time()) if we didn't want to keep the same seed for multiple runs.

Feedback from others?

Thanks.

I've decided to leave the srand call in for group allocation. The form will keep shuffling the users in the same order whenever you reload it for other unrelated changes. This seems reasonable and could be adjusted at another time if required.

thanks, submitting for integration

Hi, thanks for working on this guys.
My only comment is about a change in lib/adodb/adodb.inc.php - this is a part of an external library. We either should not change it or reflect the changes in lib/adodb/readme_moodle.txt
Just had a chat with Petr and he said: "+1 for the readme_moodle.txt together with upstream report"
Russell, can you please add a line in lib/adodb/readme_moodle.txt ?
Thanks

Comment has been added as requested. I also back-patched this to 2.6, 2.5 and 2.4 as it's causing failures for users working on those branches. As it's not a security issue, 2.3 has been left out.

Thanks Russell, integrated to master, 26, 25 and 24

Looking good on mac on 24, 25, 26 and master

I've tested on linux / mac and windows on 5.3 and 5.4. I haven't tested on windows 5.5, but i'm using integrators privilege to pass it as it seems there isn't a readily available install here and i've got a lot of other things on.

Twas the week before Christmas,
And all though HQ
Devs were scrambling to finish peer review.
They sent all their issues,
and rushed out the door -
"To the beach!" someone heard them roar!

This issue has been released upstream. Thanks!