From 5b506ab51865db3c6ea363462fc31c50d10647a3 Mon Sep 17 00:00:00 2001
From: Bernhard Posselt <dev@bernhard-posselt.com>
Date: Fri, 20 Feb 2015 13:10:16 +0100
Subject: [PATCH] use flock instead of just checking if there is a file to
 prevent deadlocks when the process crashed or the unlink wasnt done

move cron.lock into top directory to prevent external storage issues

use tmp directory lock file

use new config interface

incorporate #13770

add dots
---
 config/config.sample.php | 14 ++++++--
 cron.php                 | 73 ++++++++++++++++------------------------
 2 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/config/config.sample.php b/config/config.sample.php
index 061f368b20..10b079caa8 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -55,7 +55,7 @@ $CONFIG = array(
  *
  * @deprecated This salt is deprecated and only used for legacy-compatibility, developers
  * should *NOT* use this value for anything nowadays.
- * 
+ *
  *'passwordsalt' => 'd3c944a9af095aa08f',
  */
 'passwordsalt' => '',
@@ -507,6 +507,16 @@ $CONFIG = array(
  */
 'cron_log' => true,
 
+/**
+ * Location of the lock file for cron executions can be specified here.
+ * Default is within the tmp directory. The file is named in the following way
+ *   owncloud-server-$INSTANCEID-cron.lock
+ * where $INSTANCEID is the string specified in the instanceid field.
+ * Because the cron lock file is accessed in regular intervals, it may prevent enabled disk drives from spinning down.
+ * A different location for this file can solve such issues.
+ */
+'cron.lockfile.location' => '',
+
 /**
  * Enables log rotation and limits the total size of logfiles. The default is 0,
  * or no rotation. Specify a size in bytes, for example 104857600 (100 megabytes
@@ -669,7 +679,7 @@ $CONFIG = array(
  *  - OC\Preview\StarOffice
  *  - OC\Preview\SVG
  *  - OC\Preview\TIFF
- * 
+ *
  * .. note:: Troubleshooting steps for the MS Word previews are available
  *    at the :doc:`collaborative_documents_configuration` section
  *    of the Administrators Manual.
diff --git a/cron.php b/cron.php
index 9d0493f016..5ab0c2aa4a 100644
--- a/cron.php
+++ b/cron.php
@@ -27,29 +27,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
-// Unfortunately we need this class for shutdown function
-class TemporaryCronClass {
-	public static $sent = false;
-	public static $lockfile = "";
-	public static $keeplock = false;
-}
-
-// We use this function to handle (unexpected) shutdowns
-function handleUnexpectedShutdown() {
-	// Delete lockfile
-	if (!TemporaryCronClass::$keeplock && file_exists(TemporaryCronClass::$lockfile)) {
-		unlink(TemporaryCronClass::$lockfile);
-	}
-
-	// Say goodbye if the app did not shutdown properly
-	if (!TemporaryCronClass::$sent) {
-		if (OC::$CLI) {
-			echo 'Unexpected error!' . PHP_EOL;
-		} else {
-			OC_JSON::error(array('data' => array('message' => 'Unexpected error!')));
-		}
-	}
-}
 
 try {
 
@@ -75,15 +52,11 @@ try {
 		exit(0);
 	}
 
-	// Handle unexpected errors
-	register_shutdown_function('handleUnexpectedShutdown');
-
 	\OC::$server->getTempManager()->cleanOld();
 
 	// Exit if background jobs are disabled!
-	$appmode = OC_BackgroundJob::getExecutionType();
-	if ($appmode == 'none') {
-		TemporaryCronClass::$sent = true;
+	$appMode = OC_BackgroundJob::getExecutionType();
+	if ($appMode == 'none') {
 		if (OC::$CLI) {
 			echo 'Background Jobs are disabled!' . PHP_EOL;
 		} else {
@@ -93,35 +66,49 @@ try {
 	}
 
 	if (OC::$CLI) {
-		// Create lock file first
-		TemporaryCronClass::$lockfile = OC_Config::getValue("datadirectory", OC::$SERVERROOT . '/data') . '/cron.lock';
+		$config = OC::$server->getConfig();
+		$instanceId = $config->getSystemValue('instanceid');
+		$lockFileName = 'owncloud-server-' . $instanceId . '-cron.lock';
+		$lockDirectory = $config->getSystemValue('cron.lockfile.location', sys_get_temp_dir());
+		$lockDirectory = rtrim($lockDirectory, '\\/');
+		$lockFile = $lockDirectory . '/' . $lockFileName;
+
+		if (!file_exists($lockFile)) {
+			touch($lockFile);
+		}
 
 		// We call ownCloud from the CLI (aka cron)
-		if ($appmode != 'cron') {
-			// Use cron in future!
+		if ($appMode != 'cron') {
 			OC_BackgroundJob::setExecutionType('cron');
 		}
 
-		// check if backgroundjobs is still running
-		if (file_exists(TemporaryCronClass::$lockfile)) {
-			TemporaryCronClass::$keeplock = true;
-			TemporaryCronClass::$sent = true;
+		// open the file and try to lock if. If it is not locked, the background
+		// job can be executed, otherwise another instance is already running
+		$fp = fopen($lockFile, 'w');
+		$isLocked = flock($fp, LOCK_EX|LOCK_NB, $wouldBlock);
+
+		// check if backgroundjobs is still running. The wouldBlock check is
+		// needed on systems with advisory locking, see
+		// http://php.net/manual/en/function.flock.php#45464
+		if (!$isLocked || $wouldBlock) {
 			echo "Another instance of cron.php is still running!" . PHP_EOL;
 			exit(1);
 		}
 
-		// Create a lock file
-		touch(TemporaryCronClass::$lockfile);
-
 		// Work
 		$jobList = \OC::$server->getJobList();
 		$jobs = $jobList->getAll();
 		foreach ($jobs as $job) {
 			$job->execute($jobList, $logger);
 		}
+
+		// unlock the file
+		flock($fp, LOCK_UN);
+		fclose($fp);
+
 	} else {
 		// We call cron.php from some website
-		if ($appmode == 'cron') {
+		if ($appMode == 'cron') {
 			// Cron is cron :-P
 			OC_JSON::error(array('data' => array('message' => 'Backgroundjobs are using system cron!')));
 		} else {
@@ -136,11 +123,9 @@ try {
 		}
 	}
 
-	// done!
-	TemporaryCronClass::$sent = true;
 	// Log the successful cron execution
 	if (\OC::$server->getConfig()->getSystemValue('cron_log', true)) {
-		\OC::$server->getAppConfig()->setValue('core', 'lastcron', time());
+		\OC::$server->getConfig()->setAppValue('core', 'lastcron', time());
 	}
 	exit();