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

1 files changed, 29 insertions, 44 deletions

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();