jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/376874 )
Change subject: Inject dependencies into Shell\Command
......................................................................
Inject dependencies into Shell\Command
This slightly changes how execution time limits fall back on each other.
Change-Id: I7754a9e6be9638eebe90cb953adb8e2a6ee97cef
---
M RELEASE-NOTES-1.30
M includes/shell/Command.php
M includes/shell/Shell.php
3 files changed, 67 insertions(+), 25 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30
index 2090ce9..bec7b86 100644
--- a/RELEASE-NOTES-1.30
+++ b/RELEASE-NOTES-1.30
@@ -233,7 +233,10 @@
* DB_SLAVE is deprecated. DB_REPLICA should be used instead.
* wfUsePHP() is deprecated.
* wfFixSessionID() was removed.
-* wfShellExec() and related functions are deprecated, use Shell::command().
+* wfShellExec() and related functions are deprecated, use Shell::command().
This also
+ slightly changes the behavior of how execution time limits are calculated
when only
+ some of defaults are overridden per-call. When in doubt, always override
both wall
+ clock and CPU time.
* (T138166) SpecialEmailUser::getTarget() now requires a second argument, the
sending
user object. Using the method without the second argument is deprecated.
* (T67297) Browsers that don't support Unicode will have their edits rejected.
diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 864e69a..fd8f6a0 100644
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -24,6 +24,8 @@
use MediaWiki\ProcOpenError;
use MediaWiki\ShellDisabledError;
use Profiler;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
/**
* Class used for executing shell commands
@@ -31,11 +33,22 @@
* @since 1.30
*/
class Command {
+ use LoggerAwareTrait;
+
/** @var string */
private $command = '';
/** @var array */
- private $limits = [];
+ private $limits = [
+ // seconds
+ 'time' => 180,
+ // seconds
+ 'walltime' => 180,
+ // KB
+ 'memory' => 307200,
+ // KB
+ 'filesize' => 102400,
+ ];
/** @var string[] */
private $env = [];
@@ -49,13 +62,20 @@
/** @var bool */
private $everExecuted = false;
+ /** @var string|false */
+ private $cGroup = false;
+
/**
* Constructor. Don't call directly, instead use Shell::command()
+ *
+ * @throws ShellDisabledError
*/
public function __construct() {
if ( Shell::isDisabled() ) {
throw new ShellDisabledError();
}
+
+ $this->setLogger( new NullLogger() );
}
/**
@@ -112,11 +132,10 @@
* Sets execution limits
*
* @param array $limits Optional array with limits(filesize, memory,
time, walltime).
- * This overrides the global wgMaxShell* limits.
* @return $this
*/
public function limits( array $limits ) {
- $this->limits = $limits;
+ $this->limits = $limits + $this->limits;
return $this;
}
@@ -159,6 +178,18 @@
}
/**
+ * Sets cgroup for this command
+ *
+ * @param string|false $cgroup
+ * @return $this
+ */
+ public function cgroup( $cgroup ) {
+ $this->cGroup = $cgroup;
+
+ return $this;
+ }
+
+ /**
* Executes command. Afterwards, getExitCode() and getOutput() can be
used to access execution
* results.
*
@@ -168,8 +199,7 @@
* @throws ShellDisabledError
*/
public function execute() {
- global $IP, $wgMaxShellMemory, $wgMaxShellFileSize,
$wgMaxShellTime,
- $wgMaxShellWallClockTime, $wgShellCgroup;
+ global $IP;
$this->everExecuted = true;
@@ -197,18 +227,12 @@
$useLogPipe = false;
if ( is_executable( '/bin/bash' ) ) {
- $time = intval( isset( $this->limits['time'] ) ?
$this->limits['time'] : $wgMaxShellTime );
- if ( isset( $this->limits['walltime'] ) ) {
- $wallTime = intval( $this->limits['walltime'] );
- } elseif ( isset( $this->limits['time'] ) ) {
- $wallTime = $time;
- } else {
- $wallTime = intval( $wgMaxShellWallClockTime );
- }
- $mem = intval( isset( $this->limits['memory'] ) ?
$this->limits['memory'] : $wgMaxShellMemory );
- $filesize = intval( isset( $this->limits['filesize'] )
- ? $this->limits['filesize']
- : $wgMaxShellFileSize );
+ $time = intval( $this->limits['time'] );
+ $wallTime = intval( $this->limits['walltime'] );
+ // for b/c, wall time falls back to time
+ $wallTime = min( $time, $wallTime );
+ $mem = intval( $this->limits['memory'] );
+ $filesize = intval( $this->limits['filesize'] );
if ( $time > 0 || $mem > 0 || $filesize > 0 ||
$wallTime > 0 ) {
$cmd = '/bin/bash ' . escapeshellarg(
"$IP/includes/limit.sh" ) . ' ' .
@@ -216,7 +240,7 @@
escapeshellarg(
"MW_INCLUDE_STDERR=" . (
$this->useStderr ? '1' : '' ) . ';' .
"MW_CPU_LIMIT=$time; " .
- 'MW_CGROUP=' .
escapeshellarg( $wgShellCgroup ) . '; ' .
+ 'MW_CGROUP=' .
escapeshellarg( $this->cGroup ) . '; ' .
"MW_MEM_LIMIT=$mem; " .
"MW_FILE_SIZE_LIMIT=$filesize; " .
"MW_WALL_CLOCK_LIMIT=$wallTime; " .
@@ -252,7 +276,7 @@
$scoped = Profiler::instance()->scopedProfileIn( __FUNCTION__ .
'-' . $profileMethod );
$proc = proc_open( $cmd, $desc, $pipes );
if ( !$proc ) {
- wfDebugLog( 'exec', "proc_open() failed: $cmd" );
+ $this->logger->error( "proc_open() failed: {command}",
[ 'command' => $cmd ] );
throw new ProcOpenError();
}
$outBuffer = $logBuffer = '';
@@ -329,7 +353,7 @@
$lines = explode( "\n",
$logBuffer );
$logBuffer = array_pop( $lines
);
foreach ( $lines as $line ) {
- wfDebugLog( 'exec',
$line );
+ $this->logger->info(
$line );
}
}
}
@@ -369,7 +393,7 @@
}
if ( $logMsg !== false ) {
- wfDebugLog( 'exec', "$logMsg: $cmd" );
+ $this->logger->warning( "$logMsg: {command}", [
'command' => $cmd ] );
}
return new Result( $retval, $outBuffer );
diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php
index c293ff2..f2c96ae 100644
--- a/includes/shell/Shell.php
+++ b/includes/shell/Shell.php
@@ -22,6 +22,9 @@
namespace MediaWiki\Shell;
+use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
+
/**
* Executes shell commands
*
@@ -39,10 +42,10 @@
class Shell {
/**
- * Returns a new instance of this class
+ * Returns a new instance of Command class
*
- * @param string|string[] $command If string, a properly shell-escaped
command line,
- * or an array of unescaped arguments, in which case each value will
be escaped
+ * @param string|string[] $command String or array of strings
representing the command to
+ * be executed, each value will be escaped.
* Example: [ 'convert', '-font', 'font name' ] would produce
"'convert' '-font' 'font name'"
* @return Command
*/
@@ -54,6 +57,18 @@
$args = reset( $args );
}
$command = new Command();
+ $config = MediaWikiServices::getInstance()->getMainConfig();
+
+ $limits = [
+ 'time' => $config->get( 'MaxShellTime' ),
+ 'walltime' => $config->get( 'MaxShellWallClockTime' ),
+ 'memory' => $config->get( 'MaxShellMemory' ),
+ 'filesize' => $config->get( 'MaxShellFileSize' ),
+ ];
+ $command->limits( $limits );
+ $command->cgroup( $config->get( 'ShellCgroup' ) );
+ $command->setLogger( LoggerFactory::getInstance( 'exec' ) );
+
return $command->params( $args );
}
--
To view, visit https://gerrit.wikimedia.org/r/376874
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7754a9e6be9638eebe90cb953adb8e2a6ee97cef
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: MaxSem <maxsem.w...@gmail.com>
Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: MaxSem <maxsem.w...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
