Unify SQL timeouts

- Exec and lock timeouts now apply to MySQL
- Lock timeout now applies to PostgreSQL
- SQLite now uses a generic lock timeout setting which applies to all

lib/AbstractException.php

@@ -65,6 +65,7 @@ abstract class AbstractException extends \Exception {
         "Conf/Exception.fileCorrupt"                  => 10306,
         "Conf/Exception.typeMismatch"                 => 10311,
         "Conf/Exception.semanticMismatch"             => 10312,
+        "Conf/Exception.ambiguousDefault"             => 10313,
         "User/Exception.functionNotImplemented"       => 10401,
         "User/Exception.doesNotExist"                 => 10402,
         "User/Exception.alreadyExists"                => 10403,

lib/Conf.php

@@ -21,16 +21,16 @@ class Conf {
     public $dbDriver                = "sqlite3";
     /** @var boolean Whether to attempt to automatically update the database when upgrading to a new version with schema changes */
     public $dbAutoUpdate            = true;
-    /** @var \DateInterval Number of seconds to wait before returning a timeout error when connecting to a database (zero waits forever; not applicable to SQLite) */
+    /** @var \DateInterval|null Number of seconds to wait before returning a timeout error when connecting to a database (null waits forever; not applicable to SQLite) */
     public $dbTimeoutConnect        = 5.0;
-    /** @var \DateInterval Number of seconds to wait before returning a timeout error when executing a database operation (zero waits forever; not applicable to SQLite) */
+    /** @var \DateInterval|null Number of seconds to wait before returning a timeout error when executing a database operation (null waits forever; not applicable to SQLite) */
-    public $dbTimeoutExec           = 0.0;
+    public $dbTimeoutExec           = null;
+    /** @var \DateInterval|null Number of seconds to wait before returning a timeout error when acquiring a database lock (null waits forever) */
+    public $dbTimeoutLock           = 60.0;
     /** @var string|null Full path and file name of SQLite database (if using SQLite) */
     public $dbSQLite3File           = null;
     /** @var string Encryption key to use for SQLite database (if using a version of SQLite with SEE) */
     public $dbSQLite3Key            = "";
-    /** @var \DateInterval Number of seconds for SQLite to wait before returning a timeout error when trying to acquire a write lock on the database (zero does not wait) */
-    public $dbSQLite3Timeout        = 60.0;
     /** @var string Host name, address, or socket path of PostgreSQL database server (if using PostgreSQL) */
     public $dbPostgreSQLHost        = "";
     /** @var string Log-in user name for PostgreSQL database server (if using PostgreSQL) */
@@ -109,6 +109,11 @@ class Conf {
     /** @var string Space-separated list of origins from which to deny cross-origin resource sharing */
     public $httpOriginsDenied       = "";
 
+    ### OBSOLETE SETTINGS
+
+    /** @var \DateInterval|null (OBSOLETE) Number of seconds for SQLite to wait before returning a timeout error when trying to acquire a write lock on the database (zero does not wait) */
+    public $dbSQLite3Timeout        = null; // previously 60.0
+
     const TYPE_NAMES = [
         Value::T_BOOL     => "boolean",
         Value::T_STRING   => "string",
@@ -116,6 +121,12 @@ class Conf {
         VALUE::T_INT      => "integer",
         Value::T_INTERVAL => "interval",
     ];
+    const EXPECTED_TYPES = [
+        'dbTimeoutExec'    => "double",
+        'dbTimeoutLock'    => "double",
+        'dbTimeoutConnect' => "double",
+        'dbSQLite3Timeout' => "double",
+    ];
 
     protected static $types = [];
 
@@ -261,26 +272,28 @@ class Conf {
     }
 
     protected function propertyImport(string $key, $value, string $file = "") {
+        $typeName = static::$types[$key]['name'] ?? "mixed";
+        $typeConst = static::$types[$key]['const'] ?? Value::T_MIXED;
+        $nullable = (int) (bool) (static::$types[$key]['const'] & Value::M_NULL);
         try {
-            $typeName = static::$types[$key]['name'] ?? "mixed";
-            $typeConst = static::$types[$key]['const'] ?? Value::T_MIXED;
             if ($typeName === "\\DateInterval") {
                 // date intervals have special handling: if the existing value (ultimately, the default value)
                 // is an integer or float, the new value should be imported as numeric. If the new value is a string
                 // it is first converted to an interval and then converted to the numeric type if necessary
+                $mode = $nullable ? Value::M_STRICT | Value::M_NULL : Value::M_STRICT;
                 if (is_string($value)) {
-                    $value =  Value::normalize($value, Value::T_INTERVAL | Value::M_STRICT);
+                    $value =  Value::normalize($value, Value::T_INTERVAL | $mode);
                 }
-                switch (gettype($this->$key)) {
+                switch (self::EXPECTED_TYPES[$key] ?? gettype($this->$key)) {
                     case "integer":
-                        return Value::normalize($value, Value::T_INT | Value::M_STRICT);
+                        return Value::normalize($value, Value::T_INT | $mode);
                     case "double":
-                        return Value::normalize($value, Value::T_FLOAT | Value::M_STRICT);
+                        return Value::normalize($value, Value::T_FLOAT | $mode);
                     case "string":
                     case "object":
                         return $value;
                     default:
-                        throw new ExceptionType("strictFailure"); // @codeCoverageIgnore
+                        throw new Conf\Exception("ambiguousDefault", ['param' => $key]); // @codeCoverageIgnore
                 }
             }
             $value =  Value::normalize($value, $typeConst);
@@ -303,7 +316,6 @@ class Conf {
             }
             return $value;
         } catch (ExceptionType $e) {
-            $nullable = (int) (bool) (static::$types[$key] & Value::M_NULL);
             $type =  static::$types[$key]['const'] & ~(Value::M_STRICT | Value::M_DROP | Value::M_NULL | Value::M_ARRAY);
             throw new Conf\Exception("typeMismatch", ['param' => $key, 'type' => self::TYPE_NAMES[$type], 'file' => $file, 'nullable' => $nullable]);
         }

lib/Db/MySQL/Driver.php

@@ -18,7 +18,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
     const SQL_MODE = "ANSI_QUOTES,HIGH_NOT_PRECEDENCE,NO_BACKSLASH_ESCAPES,NO_ENGINE_SUBSTITUTION,PIPES_AS_CONCAT,STRICT_ALL_TABLES";
     const TRANSACTIONAL_LOCKS = false;
 
-    /** @var \mysql */
+    /** @var \mysqli */
     protected $db;
     protected $transStart = 0;
     protected $packetSize = 4194304;
@@ -48,7 +48,8 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         return [
             "SET sql_mode = '".self::SQL_MODE."'",
             "SET time_zone = '+00:00'",
-            "SET lock_wait_timeout = 1",
+            "SET lock_wait_timeout = ".self::lockTimeout(),
+            "SET max_execution_time = ".ceil(Arsse::$conf->dbTimeoutExec * 1000),
         ];
     }
 
@@ -130,7 +131,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
             try {
                 $this->exec("SET lock_wait_timeout = 1; LOCK TABLES $tables");
             } finally {
-                $this->exec("SET lock_wait_timeout = 60");
+                $this->exec("SET lock_wait_timeout = ".self::lockTimeout());
             }
         }
         return true;
@@ -141,6 +142,10 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         return true;
     }
 
+    protected static function lockTimeout(): int {
+        return (int) max(min(ceil(Arsse::$conf->dbTimeoutLock ?? 31536000), 31536000), 1);
+    }
+
     public function __destruct() {
         if (isset($this->db)) {
             $this->db->close();
@@ -157,7 +162,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
     }
 
     protected function makeConnection(string $db, string $user, string $password, string $host, int $port, string $socket) {
-        $this->db = @new \mysqli($host, $user, $password, $db, $port, $socket);
+        $this->db = mysqli_init();
+        $this->db->options(\MYSQLI_OPT_CONNECT_TIMEOUT, ceil(Arsse::$conf->dbTimeoutConnect));
+        @$this->db->real_connect($host, $user, $password, $db, $port, $socket);
         if ($this->db->connect_errno) {
             list($excClass, $excMsg, $excData) = $this->buildConnectionException($this->db->connect_errno, $this->db->connect_error);
             throw new $excClass($excMsg, $excData);

lib/Db/PostgreSQL/Driver.php

@@ -74,11 +74,13 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
     }
 
     public static function makeSetupQueries(string $schema = ""): array {
-        $timeout = ceil(Arsse::$conf->dbTimeoutExec * 1000);
+        $timeExec = is_null(Arsse::$conf->dbTimeoutExec) ? 0 : ceil(max(Arsse::$conf->dbTimeoutExec * 1000, 1));
+        $timeLock = is_null(Arsse::$conf->dbTimeoutLock) ? 0 : ceil(max(Arsse::$conf->dbTimeoutLock * 1000, 1));
         $out = [
             "SET TIME ZONE UTC",
             "SET DateStyle = 'ISO, MDY'",
-            "SET statement_timeout = '$timeout'",
+            "SET statement_timeout = '$timeExec'",
+            "SET lock_timeout = '$timeLock'",
         ];
         if (strlen($schema) > 0) {
             $schema = '"'.str_replace('"', '""', $schema).'"';

lib/Db/SQLite3/Driver.php

@@ -55,7 +55,8 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
             throw new Exception("fileCorrupt", $dbFile);
         }
         // set the timeout
-        $timeout = (int) ceil(Arsse::$conf->dbSQLite3Timeout * 1000);
+        $timeout = Arsse::$conf->dbSQLite3Timeout ?? Arsse::$conf->dbTimeoutLock; // old SQLite-specific timeout takes precedence
+        $timeout = is_null($timeout) ? PHP_INT_MAX : (int) ceil($timeout * 1000);
         $this->setTimeout($timeout);
         // set other initial options
         $this->exec("PRAGMA foreign_keys = yes");

locale/en.php

@@ -74,6 +74,8 @@ return [
             other {, or null}
         }',
     'Exception.JKingWeb/Arsse/Conf/Exception.semanticMismatch'             => 'Configuration parameter "{param}" in file "{file}" is not a valid value. Consult the documentation for possible values',
+    // indicates programming error
+    'Exception.JKingWeb/Arsse/Conf/Exception.ambiguousDefault'             => 'Preferred type of configuration parameter "{param}" could not be inferred from its default value. The parameter must be added to the Conf::EXPECTED_TYPES array',
     'Exception.JKingWeb/Arsse/Db/Exception.extMissing'                     => 'Required PHP extension for driver "{0}" not installed',
     'Exception.JKingWeb/Arsse/Db/Exception.fileMissing'                    => 'Database file "{0}" does not exist',
     'Exception.JKingWeb/Arsse/Db/Exception.fileUnreadable'                 => 'Insufficient permissions to open database file "{0}" for reading',

tests/cases/Db/BaseDriver.php

@@ -19,6 +19,7 @@ abstract class BaseDriver extends \JKingWeb\Arsse\Test\AbstractTest {
     protected $setVersion;
     protected static $conf = [
         'dbTimeoutExec' => 0.5,
+        'dbTimeoutLock' => 0.001,
         'dbSQLite3Timeout' => 0,
       //'dbSQLite3File' => "(temporary file)",
     ];