summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathaniel Catchpole2017-08-11 05:50:42 (GMT)
committerNathaniel Catchpole2017-08-11 05:50:42 (GMT)
commit69bf50fee050a25c0f758aa39a6ef3b7d763c451 (patch)
tree9190d3b4290ab94db8dd71af14f5c78c61517d98
parent10c87c672e8bb456decbd4e88573e4cb23b53e24 (diff)
Issue #2851111 by tedbow, tstoeckler, Munavijayalakshmi, dagmar, dawehner, catch, Fabianx, david_garcia, Nebel54, hchonov: fastcgi_finish_request and shutdown functions (e.g. batch) do not play nicely together
-rw-r--r--core/includes/batch.inc52
-rw-r--r--core/tests/Drupal/KernelTests/Core/Batch/BatchKernelTest.php40
2 files changed, 87 insertions, 5 deletions
diff --git a/core/includes/batch.inc b/core/includes/batch.inc
index 5d05cd5..402f195 100644
--- a/core/includes/batch.inc
+++ b/core/includes/batch.inc
@@ -47,8 +47,22 @@ function _batch_page(Request $request) {
}
}
- // Register database update for the end of processing.
+ // We need to store the updated batch information in the batch storage after
+ // processing the batch. In order for the error page to work correctly this
+ // needs to be done even in case of a PHP fatal error in which case the end of
+ // this function is never reached. Therefore we register a shutdown function
+ // to handle this case. Because with FastCGI and fastcgi_finish_request()
+ // shutdown functions are called after the HTTP connection is closed, updating
+ // the batch information in a shutdown function would lead to race conditions
+ // between consecutive requests if the batch processing continues. In case of
+ // a fatal error the processing stops anyway, so it works even with FastCGI.
+ // However, we must ensure to only update in the shutdown phase in this
+ // particular case we track whether the batch information still needs to be
+ // updated.
+ // @see _batch_shutdown()
+ // @see \Symfony\Component\HttpFoundation\Response::send()
drupal_register_shutdown_function('_batch_shutdown');
+ _batch_needs_update(TRUE);
$build = [];
@@ -70,18 +84,46 @@ function _batch_page(Request $request) {
$current_set = _batch_current_set();
$build['#title'] = $current_set['title'];
$build['content'] = _batch_progress_page();
+
+ $response = $build;
break;
case 'do':
// JavaScript-based progress page callback.
- return _batch_do();
+ $response = _batch_do();
+ break;
case 'finished':
// _batch_finished() returns a RedirectResponse.
- return _batch_finished();
+ $response = _batch_finished();
+ break;
}
- return $build;
+ if ($batch) {
+ \Drupal::service('batch.storage')->update($batch);
+ }
+ _batch_needs_update(FALSE);
+
+ return $response;
+}
+
+/**
+ * Checks whether the batch information needs to be updated in the storage.
+ *
+ * @param bool $new_value
+ * (optional) A new value to set.
+ *
+ * @return bool
+ * TRUE if the batch information needs to be updated; FALSE otherwise.
+ */
+function _batch_needs_update($new_value = NULL) {
+ $needs_update = &drupal_static(__FUNCTION__, FALSE);
+
+ if (isset($new_value)) {
+ $needs_update = $new_value;
+ }
+
+ return $needs_update;
}
/**
@@ -509,7 +551,7 @@ function _batch_finished() {
* @see drupal_register_shutdown_function()
*/
function _batch_shutdown() {
- if ($batch = batch_get()) {
+ if (($batch = batch_get()) && _batch_needs_update()) {
\Drupal::service('batch.storage')->update($batch);
}
}
diff --git a/core/tests/Drupal/KernelTests/Core/Batch/BatchKernelTest.php b/core/tests/Drupal/KernelTests/Core/Batch/BatchKernelTest.php
new file mode 100644
index 0000000..6240308
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Batch/BatchKernelTest.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace Drupal\KernelTests\Core\Batch;
+
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests batch functionality.
+ *
+ * @group Batch
+ */
+class BatchKernelTest extends KernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+
+ require_once $this->root . '/core/includes/batch.inc';
+ }
+
+ /**
+ * Tests _batch_needs_update().
+ */
+ public function testNeedsUpdate() {
+ // Before ever being called, the return value should be FALSE.
+ $this->assertEquals(FALSE, _batch_needs_update());
+
+ // Set the value to TRUE.
+ $this->assertEquals(TRUE, _batch_needs_update(TRUE));
+ // Check that without a parameter TRUE is returned.
+ $this->assertEquals(TRUE, _batch_needs_update());
+
+ // Set the value to FALSE.
+ $this->assertEquals(FALSE, _batch_needs_update(FALSE));
+ $this->assertEquals(FALSE, _batch_needs_update());
+ }
+
+}