Ensure all file handles to GeoIP databases are closed before replacing the file (#17085)

2 files changed, 32 insertions, 1 deletions

diff --git a/plugins/GeoIp2/GeoIP2AutoUpdater.php b/plugins/GeoIp2/GeoIP2AutoUpdater.php
index d144c66b5b..bacc6fd660 100644
--- a/plugins/GeoIp2/GeoIP2AutoUpdater.php
+++ b/plugins/GeoIp2/GeoIP2AutoUpdater.php
@@ -283,6 +283,7 @@ class GeoIP2AutoUpdater extends Task
             if ($isDbIpUnknownDbType) {
                 $php = new Php([$dbType => [$outputPath]]);
                 $dbFilename = $php->detectDatabaseType($dbType) . '.mmdb';
+                unset($php);
             }
         } else {
             $parts = explode(basename($filename), '.', 2);
@@ -306,6 +307,7 @@ class GeoIP2AutoUpdater extends Task
 
             try {
                 $location = $phpProvider->getLocation(array('ip' => LocationProviderGeoIp2::TEST_IP));
+                unset($phpProvider);
             } catch (\Exception $e) {
                 Log::info("GeoIP2AutoUpdater: Encountered exception when testing newly downloaded" .
                     " GeoIP 2 database: %s", $e->getMessage());
@@ -317,6 +319,13 @@ class GeoIP2AutoUpdater extends Task
                 throw new Exception(Piwik::translate('GeoIp2_ThisUrlIsNotAValidGeoIPDB'));
             }
 
+            // ensure the cached location providers do no longer block any files on windows
+            foreach (LocationProvider::$providers as $provider) {
+                if ($provider instanceof Php) {
+                    $provider->clearCachedInstances();
+                }
+            }
+
             // delete the existing GeoIP database (if any) and rename the downloaded file
             $oldDbFile = LocationProviderGeoIp2::getPathForGeoIpDatabase($dbFilename);
             if (file_exists($oldDbFile)) {
@@ -625,6 +634,7 @@ class GeoIP2AutoUpdater extends Task
                 $reader = new Reader($pathToDb);
 
                 $location = $provider->getLocation(array('ip' => LocationProviderGeoIp2::TEST_IP));
+                unset($provider, $reader);
             } catch (\Exception $e) {
                 if ($logErrors) {
                     Log::error("GeoIP2AutoUpdater: Encountered exception when performing redundant tests on GeoIP2 "
diff --git a/plugins/GeoIp2/LocationProvider/GeoIp2/Php.php b/plugins/GeoIp2/LocationProvider/GeoIp2/Php.php
index 3ff3875b42..3bf1496d02 100644
--- a/plugins/GeoIp2/LocationProvider/GeoIp2/Php.php
+++ b/plugins/GeoIp2/LocationProvider/GeoIp2/Php.php
@@ -38,7 +38,7 @@ class Php extends GeoIp2
      *
      * Each instance is mapped w/ one of the following keys: 'loc', 'isp'
      *
-     * @var array of GeoIP instances
+     * @var Reader[] of GeoIP instances
      */
     private $readerCache = array();
 
@@ -78,6 +78,11 @@ class Php extends GeoIp2
         }
     }
 
+    public function __destroy()
+    {
+        $this->clearCachedInstances();
+    }
+
     /**
      * Uses a GeoIP 2 database to get a visitor's location based on their IP address.
      *
@@ -484,6 +489,22 @@ class Php extends GeoIp2
     }
 
     /**
+     * Clears the cached instances and releases the file handles
+     */
+    public function clearCachedInstances()
+    {
+        if (empty($this->readerCache)) {
+            return;
+        }
+
+        foreach ($this->readerCache as $reader) {
+            $reader->close();
+        }
+
+        unset($this->readerCache);
+    }
+
+    /**
      * Returns a GeoIP2 reader instance. Creates it if necessary.
      *
      * @param string $key 'loc' or 'isp'. Determines the type of GeoIP database