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

2 files changed, 41 insertions, 46 deletions

diff --git a/config/config.sample.php b/config/config.sample.php
index 061f368b20e..10b079caa8a 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' => '',
@@ -508,6 +508,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
  * = 100 * 1024 * 1024 bytes). A new logfile is created with a new name when the
@@ -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 9d0493f016e..5ab0c2aa4af 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();