Merge branch 'legacy/pm4' into stable

src/scheduler/AsyncTask.php

@@ -244,9 +244,15 @@ abstract class AsyncTask extends Runnable{
 	final public function __destruct(){
 		$this->reallyDestruct();
 		if(self::$threadLocalStorage !== null && isset(self::$threadLocalStorage[$h = spl_object_id($this)])){
-			unset(self::$threadLocalStorage[$h]);
+			//Beware changing this code!
-			if(self::$threadLocalStorage->count() === 0){
+			//This code may cause the GC to be triggered, causing destruction of other AsyncTasks (which may or may not
+			//have been indirectly referenced by the TLS).
+			//This may cause the code to be re-entered from a different context unexpectedly, causing a crash if handled
+			//incorrectly.
+			if(self::$threadLocalStorage->count() === 1){
 				self::$threadLocalStorage = null;
+			}else{
+				unset(self::$threadLocalStorage[$h]);
 			}
 		}
 	}

tests/phpunit/scheduler/AsyncPoolTest.php

@@ -89,4 +89,36 @@ class AsyncPoolTest extends TestCase{
 			usleep(50 * 1000);
 		}
 	}
+
+	/**
+	 * This test ensures that the fix for an exotic AsyncTask::__destruct() reentrancy bug has not regressed.
+	 *
+	 * Due to an unset() in the function body, other AsyncTask::__destruct() calls could be triggered during
+	 * an AsyncTask's destruction. If done in the wrong way, this could lead to a crash.
+	 *
+	 * @doesNotPerformAssertions This test is checking for a crash condition, not a specific output.
+	 */
+	public function testTaskDestructorReentrancy() : void{
+		$this->pool->submitTask(new class extends AsyncTask{
+			public function __construct(){
+				$this->storeLocal("task", new class extends AsyncTask{
+
+					public function __construct(){
+						$this->storeLocal("dummy", 1);
+					}
+
+					public function onRun() : void{
+						//dummy
+					}
+				});
+			}
+
+			public function onRun() : void{
+				//dummy
+			}
+		});
+		while($this->pool->collectTasks()){
+			usleep(50 * 1000);
+		}
+	}
 }