From 074ce9ab1461aa0dc7bd18d65dc3c1b60061f571 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Fri, 15 Mar 2019 00:05:52 +0000
Subject: [PATCH] User: Remove redundant wgClockSkewFudge code

Ensuring the new value is at least as high as 1 second higher
than the current value is sufficient.

The main code paths using this are checkAndSetTouched (for user group
changes) and saveSettings(), both of which use makeUpdateConditions() which
ensures we bail out if something else already wrote to it in the mean time.
As such, there is no longer a need to make sure our time is higher than
something another server may have written, given that is no longer something
we support.

This variable was introduced in 2005 (MW 1.4) with r9403 (1d12276bcb3),
and factored out as newTouchedTimestamp() in 2007 (MW 1.8)
with r16772 (c1094ba9876).

Change-Id: I940fb0dd125286a4a348c11e2c8d197f9288a75d
---
 RELEASE-NOTES-1.33           |  6 ++++++
 includes/DefaultSettings.php |  8 --------
 includes/user/User.php       | 11 +++++------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index d3001f3e59..79b457c77e 100644
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -45,6 +45,12 @@ production.
   to configure a file in which to cache the SiteStore database table.
   This was never used. SiteStore already caches its information by default
   using BagOStuff (e.g. Memcached or APC).
+* $wgClockSkewFudge has been removed. It was used by User.php to minimize the
+  chances of a user.user_touched database update to the "current" timestamp
+  being before the value already there (e.g. due to clock skew between different
+  servers). This is no longer a problem because the code now ensures the
+  timestamp is always higher than the previous one. The writes are guarded with
+  CAS logic (check and set), which prevents updates that would overlap.
 
 === New features in 1.33 ===
 * (T96041) __EXPECTUNUSEDCATEGORY__ on a category page causes the category
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index a215af57a7..68d784614a 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2677,14 +2677,6 @@ $wgSidebarCacheExpiry = 86400;
  */
 $wgUseGzip = false;
 
-/**
- * Clock skew or the one-second resolution of time() can occasionally cause cache
- * problems when the user requests two pages within a short period of time. This
- * variable adds a given number of seconds to vulnerable timestamps, thereby giving
- * a grace period.
- */
-$wgClockSkewFudge = 5;
-
 /**
  * Invalidate various caches when LocalSettings.php changes. This is equivalent
  * to setting $wgCacheEpoch to the modification time of LocalSettings.php, as
diff --git a/includes/user/User.php b/includes/user/User.php
index 277731a0e5..ea72bdace9 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -2762,17 +2762,16 @@ class User implements IDBAccessObject, UserIdentity {
 	/**
 	 * Generate a current or new-future timestamp to be stored in the
 	 * user_touched field when we update things.
+	 *
 	 * @return string Timestamp in TS_MW format
 	 */
 	private function newTouchedTimestamp() {
-		global $wgClockSkewFudge;
-
-		$time = wfTimestamp( TS_MW, time() + $wgClockSkewFudge );
-		if ( $this->mTouched && $time <= $this->mTouched ) {
-			$time = wfTimestamp( TS_MW, wfTimestamp( TS_UNIX, $this->mTouched ) + 1 );
+		$time = time();
+		if ( $this->mTouched ) {
+			$time = max( $time, wfTimestamp( TS_UNIX, $this->mTouched ) + 1 );
 		}
 
-		return $time;
+		return wfTimestamp( TS_MW, $time );
 	}
 
 	/**
-- 
2.20.1