Merge pull request #14400 from owncloud/fix-cron-deadlock

Use flock instead of just checking if there is a file to prevent deadloc...

config/config.sample.php

@@ -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

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();
+	$appMode = OC_BackgroundJob::getExecutionType();
-	if ($appmode == 'none') {
+	if ($appMode == 'none') {
-		TemporaryCronClass::$sent = true;
 		if (OC::$CLI) {
 			echo 'Background Jobs are disabled!' . PHP_EOL;
 		} else {
@@ -93,35 +66,49 @@ try {
 	}
 
 	if (OC::$CLI) {
-		// Create lock file first
+		$config = OC::$server->getConfig();
-		TemporaryCronClass::$lockfile = OC_Config::getValue("datadirectory", OC::$SERVERROOT . '/data') . '/cron.lock';
+		$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') {
+		if ($appMode != 'cron') {
-			// Use cron in future!
 			OC_BackgroundJob::setExecutionType('cron');
 		}
 
-		// check if backgroundjobs is still running
+		// open the file and try to lock if. If it is not locked, the background
-		if (file_exists(TemporaryCronClass::$lockfile)) {
+		// job can be executed, otherwise another instance is already running
-			TemporaryCronClass::$keeplock = true;
+		$fp = fopen($lockFile, 'w');
-			TemporaryCronClass::$sent = true;
+		$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();