From 792dc3c74a01174ee7ce650be2ae09ad9d65f9c2 Mon Sep 17 00:00:00 2001
From: Tim Starling <tstarling@wikimedia.org>
Date: Fri, 15 Jun 2018 20:31:02 +1000
Subject: [PATCH] Improve timeouts in MultiHttpClient

* PHP 7 only checks for request/connection timeout after the timeout
  passed to curl_multi_select() expires. So it's necessary to use a
  select timeout which is much shorter than 10s. I filed
  https://bugs.php.net/bug.php?id=76480 for this.
* Use millisecond resolution timeout parameters. These have been
  available since libcurl 7.16.2, released Jan 2008, available in
  Debian 7, Ubuntu 14.04 and CentOS 6.

Change-Id: Ia07b824dde179b33e14b81a76d580ce547bca315
---
 includes/libs/MultiHttpClient.php | 43 ++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/includes/libs/MultiHttpClient.php b/includes/libs/MultiHttpClient.php
index cb60b019e4..20ddf723f9 100644
--- a/includes/libs/MultiHttpClient.php
+++ b/includes/libs/MultiHttpClient.php
@@ -50,9 +50,9 @@ class MultiHttpClient implements LoggerAwareInterface {
 	protected $multiHandle = null; // curl_multi handle
 	/** @var string|null SSL certificates path */
 	protected $caBundlePath;
-	/** @var int */
+	/** @var float */
 	protected $connTimeout = 10;
-	/** @var int */
+	/** @var float */
 	protected $reqTimeout = 300;
 	/** @var bool */
 	protected $usePipelining = false;
@@ -65,6 +65,11 @@ class MultiHttpClient implements LoggerAwareInterface {
 	/** @var LoggerInterface */
 	protected $logger;
 
+	// In PHP 7 due to https://bugs.php.net/bug.php?id=76480 the request/connect
+	// timeouts are periodically polled instead of being accurately respected.
+	// The select timeout is set to the minimum timeout multiplied by this factor.
+	const TIMEOUT_ACCURACY_FACTOR = 0.1;
+
 	/**
 	 * @param array $options
 	 *   - connTimeout     : default connection timeout (seconds)
@@ -148,6 +153,8 @@ class MultiHttpClient implements LoggerAwareInterface {
 	public function runMulti( array $reqs, array $opts = [] ) {
 		$chm = $this->getCurlMulti();
 
+		$selectTimeout = $this->getSelectTimeout( $opts );
+
 		// Normalize $reqs and add all of the required cURL handles...
 		$handles = [];
 		foreach ( $reqs as $index => &$req ) {
@@ -224,7 +231,7 @@ class MultiHttpClient implements LoggerAwareInterface {
 				} while ( $mrc == CURLM_CALL_MULTI_PERFORM );
 				// Wait (if possible) for available work...
 				if ( $active > 0 && $mrc == CURLM_OK ) {
-					if ( curl_multi_select( $chm, 10 ) == -1 ) {
+					if ( curl_multi_select( $chm, $selectTimeout ) == -1 ) {
 						// PHP bug 63411; https://curl.haxx.se/libcurl/c/curl_multi_fdset.html
 						usleep( 5000 ); // 5ms
 					}
@@ -285,11 +292,11 @@ class MultiHttpClient implements LoggerAwareInterface {
 	protected function getCurlHandle( array &$req, array $opts = [] ) {
 		$ch = curl_init();
 
-		curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT,
-			$opts['connTimeout'] ?? $this->connTimeout );
+		curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT_MS,
+			( $opts['connTimeout'] ?? $this->connTimeout ) * 1000 );
 		curl_setopt( $ch, CURLOPT_PROXY, $req['proxy'] ?? $this->proxy );
-		curl_setopt( $ch, CURLOPT_TIMEOUT,
-			$opts['reqTimeout'] ?? $this->reqTimeout );
+		curl_setopt( $ch, CURLOPT_TIMEOUT_MS,
+			( $opts['reqTimeout'] ?? $this->reqTimeout ) * 1000 );
 		curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 1 );
 		curl_setopt( $ch, CURLOPT_MAXREDIRS, 4 );
 		curl_setopt( $ch, CURLOPT_HEADER, 0 );
@@ -410,6 +417,28 @@ class MultiHttpClient implements LoggerAwareInterface {
 		return $ch;
 	}
 
+	/**
+	 * Get a suitable select timeout for the given options.
+	 *
+	 * @param array $opts
+	 * @return float
+	 */
+	private function getSelectTimeout( $opts ) {
+		$connTimeout = $opts['connTimeout'] ?? $this->connTimeout;
+		$reqTimeout = $opts['reqTimeout'] ?? $this->reqTimeout;
+		$timeouts = array_filter( [ $connTimeout, $reqTimeout ] );
+		if ( count( $timeouts ) === 0 ) {
+			return 1;
+		}
+
+		$selectTimeout = min( $timeouts ) * self::TIMEOUT_ACCURACY_FACTOR;
+		// Minimum 10us for sanity
+		if ( $selectTimeout < 10e-6 ) {
+			$selectTimeout = 10e-6;
+		}
+		return $selectTimeout;
+	}
+
 	/**
 	 * @return resource
 	 * @throws Exception
-- 
2.20.1