The PID we store for the Hydroponics Service is the one of the Parent process, killing it leaves children

[DiegoPino]

What?

@giancarlobi @patdunlavey @aksm this code here:

strawberryfield/src/StrawberryfieldHydroponicsService.php

Lines 170 to 213 in 911f8cb

	if ($config->get('active')) {
	global $base_url;
	$site_path = \Drupal::service('site.path'); // e.g.: 'sites/default'
	$site_path = explode('/', $site_path);
	$site_name = $site_path[1];
	$queuerunner_pid = (int) \Drupal::state()->get('hydroponics.queurunner_last_pid', 0);
	$lastRunTime = intval(\Drupal::state()->get('hydroponics.heartbeat'));
	$currentTime = intval(\Drupal::time()->getRequestTime());
	$running_posix = posix_kill($queuerunner_pid, 0);
	if (!$running_posix || !$queuerunner_pid) {
	$this->logger->info('Hydroponics Service Not running, starting, time passed since last seen @time', [
	'@time' => ($currentTime - $lastRunTime)]
	);
	$path = $config->get('drush_path');
	if (empty($path)) {
	$path = '/var/www/html/vendor/drush/drush/drush';
	}
	$path = escapeshellcmd($path);
	$cmd = $path.' archipelago:hydroponics --quiet --uri=' . $base_url;
	$home = $config->get('home_path');
	if (!empty($home)) {
	$home = escapeshellcmd($home);
	$cmd = "export HOME='".$home."'; ".$cmd;
	}
	$pid = exec(
	sprintf("%s > /dev/null 2>&1 & echo $!", $cmd)
	);
	\Drupal::state()->set('hydroponics.queurunner_last_pid', $pid);
	$this->logger->info('PID for Hydroponics Service: @pid', [
	'@pid' => $pid]
	);
	} else {
	$this->logger->info('Hydroponics Service already running with PID @pid, time passed since last seen @time', [
	'@time' => ($currentTime - $lastRunTime),
	'@pid' => $queuerunner_pid
	]
	);
	}
	}
	else {
	return;
	}
	}

Seems to be getting the PID of the sh that calls the command itself, but not the child process that is started. When we kill it, we are leaving a Child behind that eventually dies but gives this error

php-fpm "oops, unknown child exited with 0".

I think we should try with proc_open instead of exec? Or, save not only the PID of the Parent Process but also of the child one. Then when we need to kill it, we try first a SIGTERM? If that does not kill both (parent and child) we try with the Child first, then the parent?

This is not a very big deal but I would love to see those errors gone, might end generating a slow down eventually (and a lot of php-fpm processes) on heavy load.

[DiegoPino]

There are some documented bugs here regarding this
https://bugs.php.net/bug.php?id=73342

Seems like we could do this (like we do on SBR)
Because the parent process detaches eventually we could close it... but this needs testing

$command = 'command to execute...';
$pipes = array();
$process = proc_open(
  $command,
  array(
     0 => array('pipe', 'r'),
     1 => array('pipe', 'w'),
     2 => array('pipe', 'w'),
  ),
  $pipes
);

stream_set_blocking($pipes[1], true);
stream_set_blocking($pipes[2], true);

if (
  is_resource($process)
) {

  $output = stream_get_contents($pipes[1]);
  $error = stream_get_contents($pipes[2]);

  fclose($pipes[1]);
  fclose($pipes[2]);

  proc_close($process);

}

[DiegoPino]

https://github.com/esmero/strawberry_runners/blob/dbac9cf07d910dcef3f26d115ced1d5bd774e377/src/Plugin/StrawberryRunnersPostProcessorPluginBase.php#L197-L232

[patdunlavey]

This may be related to ISSUE-75 (issue, pull-request). We found that on large background ocr queues we were getting multiple processes running the same ghostscript and tesseract commands, and this could possibly be the result of \Drupal\strawberry_runners\Plugin\StrawberryRunnersPostProcessorPluginBase::proc_execute failing to close processes due to having incorrect process ids.

[DiegoPino]

@patdunlavey this is not what I observed today. What I observed is you were running Hydroponics via Drush multiple times and they competing and using all resources. Is this a new issue I did not see?

[patdunlavey]

I didn't notice that this issue was being reported for the strawberryfield module, not strawberry_runners, and that it proposes to solve it using an approach implemented in strawberry_runners. Apologies for the confusion!

[DiegoPino]

Still pending.