Last modified: 2014-02-12 23:32:54 UTC
Created attachment 12190 [details] make ulimit4.sh /bin/sh aware $wgMaxShellMemory isn't respected on POSIX aware Operating Systems other than Linux. In includes/GlobalFunctions.php around line 2785 we have <code>if ( php_uname( 's' ) == 'Linux' ) {</code> around the handling of using "ulimit4.sh" Calling "convert" without any limits set will kill your server. also see: http://www.mediawiki.org/wiki/Manual_talk:$wgMaxShellMemory#Caution:_This_is_LINUX_only.21_Won.27t_work_with_POSIX_like_systems_e.g._FreeBSD I patched this on my MW-1.20.3 and it seems fine: * https://wiki.uugrn.org/patches/ulimit4.sh.patch * https://wiki.uugrn.org/patches/GlobalFunctions.php.patch Please review this patches. I set severity to "major" as this will kill wikis running on FreeBSD by just uploading one complex/broken SVG. Regards Raphael
Created attachment 12191 [details] Make wfShellExec() FreeBSD aware.
Changing component to general. Not specificly a file management issue
ulimit4.sh is no longer in current master code. You may want to test our current code and see whether it works in your environment.
(In reply to comment #3) > ulimit4.sh is no longer in current master code. You may want to test our > current code and see whether it works in your environment. However, wfShellExec() should not only work with Linux and /bin/bash when dealing with process limits. Please provide some pointer how this will be done in future releases. Regards Raphael
Again this is not about File management, which refers to files uploaded by users. A similar script is now at includes/limit.sh . See https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/limit.sh;hb=refs/heads/master .
"ulimit -v" is not ideal for memory limiting, and was only used due to limitations of the Linux kernel. We've now implemented support for cgroups, which is a far superior option for Linux. Maybe for FreeBSD, -m (RSS) should be used, with -w (swap size) set to zero.
(In reply to comment #6) > "ulimit -v" is not ideal for memory limiting, and was only used due to > limitations of the Linux kernel. We've now implemented support for cgroups, > which is a far superior option for Linux. > > Maybe for FreeBSD, -m (RSS) should be used, with -w (swap size) set to zero. * Everything using hardcoded "/bin/bash" will break with non-Linux OS'es. * Everything using hardcoded non-standard path/tool will break on most standard OS'es.
(In reply to comment #7) > * Everything using hardcoded "/bin/bash" will break with non-Linux OS'es. > * Everything using hardcoded non-standard path/tool will break on most > standard > OS'es. Update them so they aren't hardcoded?
There SHOULD be an alternative method to cgroups by using POSIX ulimit. Better have ulimits set instead of nothing. Maybe the method is decided in one global limit.sh or in PHP depending on OS or $wgWhateverLimitMethod="ulimit" configuration Using timeout is cool stuff, but the executable in FreeBSD is located at /usr/local/bin/gtimeout (provided by coreutils-8.20_2 in my case). The script SHOULD check for the existence of this binary. If there's really the need of a (modern) bash, then make /bin/bash variable, e.g. /usr/local/bin/bash My bugreport above matches 1.20.3 and will match in another way when using the new cool cgroups stuff AFAICS here. Regards Raphael
(In reply to comment #7) > * Everything using hardcoded "/bin/bash" will break with non-Linux OS'es. > * Everything using hardcoded non-standard path/tool will break on most > standard > OS'es. I'm aware of that. That's why wfShellExec() doesn't attempt to use the script on any operating system other than Linux. I think FreeBSD support would be a new feature; its absence is not a bug. I'm trying to work out how that feature should best be implemented. I thought you might have some useful information on that, since you are evidently a FreeBSD user. Note that support for memory limiting on Mac OS X and Windows is also absent.
Hi Tim, my 2 patches (against ulimit4.sh and wfShellExec()) should make this code Linux-*and*-BSD aware (I guess it should work this way on any POSIX compatible system with /bin/sh). The main difference is, that /bin/sh ulimit cannot set multiple resource limits at once. Setting 3 limits require 3x ulimit. That's all. If you do 3x ulimit instead of one long ulimit command, this should work with (any?) POSIX-/bin/sh so you might replace /bin/bash with /bin/sh generally. FreeBSD doesn't implement cgroups, so using "good old ulimit" is better than nothing here. Your new limit.sh might "detect" this inability of cgroups and may fall back to "traditional ulimits" (like ulimit4.sh in my patch). This would be great. I cannot send you patches for your new code yet because I have no "unstable" mediawiki installed anywhere, I just cannot fix such things. Regards Raphael
Raphael: Any chance to put these patches into Gerrit?
Raphael: Could you put these patches into Gerrit? See https://www.mediawiki.org/wiki/Developer_access and to submit this as a Git branch directly into Gerrit: https://www.mediawiki.org/wiki/Git/Tutorial Putting your branch in Git makes it easier to review it quickly.
