platform: re-scope set_resource_limits

Rename 'set_resource_limits' to 'maximize_open_file_limit', as this
seems to more accurately reflect it's function.

Remove unused flexibility from the implementation, to make it easier to
understand.

Simplify the Mac implementation, and add some explanation for the
OPEN_MAX magic number. In later work we should remove the magic number.

Import 'resource' late, which is not available on Windows.

2 files changed, 32 insertions, 13 deletions

diff --git a/src/buildstream/_platform/darwin.py b/src/buildstream/_platform/darwin.py
index 8e08685ec..282a5b445 100644
--- a/src/buildstream/_platform/darwin.py
+++ b/src/buildstream/_platform/darwin.py
@@ -16,6 +16,7 @@
 #  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import resource
 
 from ..sandbox import SandboxDummy
 
@@ -44,5 +45,20 @@ class Darwin(Platform):
         else:
             return min(cpu_count, cap)
 
-    def set_resource_limits(self, soft_limit=OPEN_MAX, hard_limit=None):
-        super().set_resource_limits(soft_limit)
+    def maximize_open_file_limit(self):
+        # Note that on Mac OSX, you may not be able to simply set the soft
+        # limit to the reported hard limit, as it may not be the only limit in
+        # effect. The output of these commands may be somewhat independent:
+        #
+        #   $ launchctl limit
+        #   $ sysctl -a | grep files
+        #
+        # The OPEN_MAX value from syslimits.h seems to be fairly safe, although
+        # users may tweak their individual systems to have different values.
+        # Without a way to determine what the real limit is, we risk failing to
+        # increase the limit. Perhaps the complication is why psutil does not
+        # support rlimit on Mac.
+        #
+        old_soft_limit, hard_limit = resource.getrlimit(resource.RLIMIT_NOFILE)
+        soft_limit = min(max(self.OPEN_MAX, old_soft_limit), hard_limit)
+        resource.setrlimit(resource.RLIMIT_NOFILE, (soft_limit, hard_limit))
diff --git a/src/buildstream/_platform/platform.py b/src/buildstream/_platform/platform.py
index 0fbe145f9..5f2b7081a 100644
--- a/src/buildstream/_platform/platform.py
+++ b/src/buildstream/_platform/platform.py
@@ -20,7 +20,6 @@
 import os
 import platform
 import sys
-import resource
 
 import psutil
 
@@ -36,7 +35,7 @@ class Platform():
     # sandbox factory as well as platform helpers.
     #
     def __init__(self):
-        self.set_resource_limits()
+        self.maximize_open_file_limit()
 
     @classmethod
     def _create_instance(cls):
@@ -153,14 +152,18 @@ class Platform():
         raise ImplError("Platform {platform} does not implement check_sandbox_config()"
                         .format(platform=type(self).__name__))
 
-    def set_resource_limits(self, soft_limit=None, hard_limit=None):
+    def maximize_open_file_limit(self):
         # Need to set resources for _frontend/app.py as this is dependent on the platform
         # SafeHardlinks FUSE needs to hold file descriptors for all processes in the sandbox.
-        # Avoid hitting the limit too quickly.
-        limits = resource.getrlimit(resource.RLIMIT_NOFILE)
-        if limits[0] != limits[1]:
-            if soft_limit is None:
-                soft_limit = limits[1]
-            if hard_limit is None:
-                hard_limit = limits[1]
-            resource.setrlimit(resource.RLIMIT_NOFILE, (soft_limit, hard_limit))
+        # Avoid hitting the limit too quickly, by increasing it as far as we can.
+
+        # Import this late, as it is not available on Windows. Note that we
+        # could use `psutil.Process().rlimit` instead, but this would introduce
+        # a dependency on the `prlimit(2)` call, which seems to only be
+        # available on Linux. For more info:
+        # https://github.com/giampaolo/psutil/blob/cbf2bafbd33ad21ef63400d94cb313c299e78a45/psutil/_psutil_linux.c#L45
+        import resource
+
+        soft_limit, hard_limit = resource.getrlimit(resource.RLIMIT_NOFILE)
+        if soft_limit != hard_limit:
+            resource.setrlimit(resource.RLIMIT_NOFILE, (hard_limit, hard_limit))