Calling 'new Date()' more than once in an init function is problematic
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Evergreen |
Fix Released
|
High
|
Unassigned | ||
3.3 |
Won't Fix
|
High
|
Unassigned | ||
3.4 |
Fix Released
|
High
|
Unassigned |
Bug Description
Evergreen 3.3.6 / 3.4.2 / master
Bug 1712644 is a great fix (I look forward to never again cleaning up a circ in December that was accidentally made due a year ago!) however, it allowed a race condition to surface that's been hiding in one or more Angular initializers, in this case circ/patron/
Let's consider some code:
$scope.checkoutArgs = {
noncat_type : 'barcode',
due_date : new Date()
};
$scope.minDate = new Date();
Innocent enough, eh? Surely due_date and minDate are the same, they're called at the same time!
I've attached a little test file that just does this 10000 times in a row and tells you about every time the two times differ when comparing their whole second times. It's really rare not to get multiple differing results every time it's run. You may ask yourself what kind of weirdo calls 'new Date()' 20000 times in a row and that hurts a little bit, but I forgive you. More interesting though is what happens when 1000 circ staff open the checkout window 100 times a day; some of them are going to lose this race.
So let's fix it!
$scope.minDate = new Date();
$scope.checkoutArgs = {
noncat_type : 'barcode',
due_date : new Date()
};
Problem solved, done and dusted; minDate will be less than due_date no matter how many times you run it, git commit -as -m "Pithy message re: the inexorable march of time." And that's true so long as there's never any other date added that needs to be directly compared to either of these. Oh, and we probably better check maxDate which is also used in a few places also and make certain that compared dates are always declared in the proper order.
Or, at the top of these functions that care about more than 1 date/time we declare a single Date object for "now" and replace every use of new Date() with new Date(now) to copy the same time throughout the function. That way it doesn't matter what order your variables are declared (except that now needs to be first, but 1 positionally relevant variable is definitely better than more than one.)
One last time:
var now = new Date();
$scope.checkoutArgs = {
noncat_type : 'barcode',
due_date : new Date(now)
};
// Can't cause date validation failures anymore, even being defined after due_date
$scope.minDate = new Date(now);
Branch on its way.
Changed in evergreen: | |
importance: | Undecided → High |
Changed in evergreen: | |
status: | New → Confirmed |
milestone: | none → 3.5.0 |
Changed in evergreen: | |
milestone: | 3.5.0 → 3.5.1 |
Changed in evergreen: | |
status: | Fix Committed → Fix Released |
Here it is: https:/ /git.evergreen- ils.org/ ?p=working/ Evergreen. git;a=shortlog; h=refs/ heads/user/ jboyer/ lp1864056_ reuse_dates / working/ user/jboyer/ lp1864056_ reuse_dates
Because of the nature of the issue it turns out that the only places it can pop up at present are in the circ app of the AngularJS client, so it's a small patch.
Testing is tricky since it's hard to cause the problem to happen in the first place, but if you apply this patch and everything works as expected when checking out, renewing, and viewing items out that means that nothing was broken, anyway.