Refactor socket activation and fd inheritance #1310

Merged
merged 1 commit into from Dec 27, 2016

Conversation

Projects
None yet
3 participants
@tilgovi
Collaborator

tilgovi commented Jul 19, 2016

Track the use of systemd socket activation and gunicorn socket inheritance
in the arbiter. Unify the logic of creating gunicorn sockets from each of
these sources to always use the socket name to determine the type rather
than checking the configured addresses. The configured addresses are only
used when there is no inheritance from systemd or a parent arbiter.

Fix #1298

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Jul 19, 2016

Collaborator

Could use additional tests for socket creation, especially inheritance and systemd activation. I only really added tests that assert unlink is done correctly.

Collaborator

tilgovi commented Jul 19, 2016

Could use additional tests for socket creation, especially inheritance and systemd activation. I only really added tests that assert unlink is done correctly.

tests/test_sock.py
+ listener1.getsockname.return_value = '/var/run/test.sock'
+ sock.close_sockets([listener1], False)
+ listener1.close.assert_called_with()
+ unlink.assert_not_called()

This comment has been minimized.

@berkerpeksag

berkerpeksag Jul 19, 2016

Collaborator

assert_not_called is new in Python 3.5.

@berkerpeksag

berkerpeksag Jul 19, 2016

Collaborator

assert_not_called is new in Python 3.5.

This comment has been minimized.

@tilgovi

tilgovi Jul 20, 2016

Collaborator

Thanks!

@tilgovi

tilgovi Jul 20, 2016

Collaborator

Thanks!

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Jul 22, 2016

Collaborator

Ping @benoitc @berkerpeksag @diwu1989

I haven't done any manual testing yet, but I added a number of unit tests around the re-exec and socket inheritance code. Ready for review.

Collaborator

tilgovi commented Jul 22, 2016

Ping @benoitc @berkerpeksag @diwu1989

I haven't done any manual testing yet, but I added a number of unit tests around the re-exec and socket inheritance code. Ready for review.

gunicorn/arbiter.py
+ environ['LISTEN_FDS'] = str(len(self.LISTENERS))
+ else:
+ fds = [l.fileno() for l in self.LISTENERS]
+ environ['GUNICORN_FD'] = ",".join([str(fd) for fd in fds])

This comment has been minimized.

@berkerpeksag

berkerpeksag Jul 23, 2016

Collaborator

Since fds is not used anywhere else, perhaps we can change it to:

environ['GUNICORN_FD'] = ",".join(str(l.fileno()) for l in self.LISTENERS)
@berkerpeksag

berkerpeksag Jul 23, 2016

Collaborator

Since fds is not used anywhere else, perhaps we can change it to:

environ['GUNICORN_FD'] = ",".join(str(l.fileno()) for l in self.LISTENERS)

This comment has been minimized.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

I try to avoid nesting, but in this case I think you're right especially because the generator expression doesn't need an extra set of parens when used as an argument to join, so the nesting isn't too terrible.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

I try to avoid nesting, but in this case I think you're right especially because the generator expression doesn't need an extra set of parens when used as an argument to join, so the nesting isn't too terrible.

This comment has been minimized.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Actually, I might just leave it as is. The line starts getting long and anyway if fileno() fails the traceback might be clearer.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Actually, I might just leave it as is. The line starts getting long and anyway if fileno() fails the traceback might be clearer.

This comment has been minimized.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Okay, I'll do it, and break it over a line :)

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Okay, I'll do it, and break it over a line :)

gunicorn/sock.py
+ sock_name = sock.getsockname()
+ sock.close()
+ if unlink and _sock_type(sock_name) is UnixSocket:
+ sock_name = sock.getsockname()

This comment has been minimized.

@berkerpeksag

berkerpeksag Jul 23, 2016

Collaborator

Can we use sock_name defined in line 199?

@berkerpeksag

berkerpeksag Jul 23, 2016

Collaborator

Can we use sock_name defined in line 199?

This comment has been minimized.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Good catch. Refactoring mistake.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

Good catch. Refactoring mistake.

gunicorn/arbiter.py
- if self.reexec_pid == 0 and self.master_pid == 0:
- for l in self.LISTENERS:
- l.close()
+ unlink = self.reexec_pid == self.master_pid and not self.systemd

This comment has been minimized.

@benoitc

benoitc Jul 23, 2016

Owner

we need to test self.reexec_pid==0 also since it should be different from the pid.

@benoitc

benoitc Jul 23, 2016

Owner

we need to test self.reexec_pid==0 also since it should be different from the pid.

This comment has been minimized.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

I don't think there's any way they can be equal unless they are zero, but I agree it would be easier to read and safer to be explicit. Fixing.

@tilgovi

tilgovi Jul 23, 2016

Collaborator

I don't think there's any way they can be equal unless they are zero, but I agree it would be easier to read and safer to be explicit. Fixing.

@tilgovi tilgovi referenced this pull request Jul 24, 2016

Open

Add Windows support #524

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Jul 24, 2016

Collaborator

I addressed all the comments.

The one thing I don't like is that I'm not setting FD_CLOEXEC. I made a note in the docstring about how this deviates from the systemd library calls. I'm thinking maybe we should remove the hack for #978 and sprinkle in PEP 446 compatibility everywhere:

diff --git a/gunicorn/arbiter.py b/gunicorn/arbiter.py
index 7296d2d..9c7cbf6 100644
--- a/gunicorn/arbiter.py
+++ b/gunicorn/arbiter.py
@@ -156,6 +156,10 @@ class Arbiter(object):

             self.LISTENERS = sock.create_sockets(self.cfg, self.log, fds)

+            # Sockets should be inheritable by a new arbiter.
+            for sock in self.LISTENERS:
+                sock.set_inheritable(True)
+
         listeners_str = ",".join([str(l) for l in self.LISTENERS])
         self.log.debug("Arbiter booted")
         self.log.info("Listening at: %s (%s)", listeners_str, self.pid)
@@ -177,12 +181,9 @@ class Arbiter(object):
             [os.close(p) for p in self.PIPE]

         # initialize the pipe
-        self.PIPE = pair = os.pipe()
-        for p in pair:
-            util.set_non_blocking(p)
-            util.close_on_exec(p)
-
-        self.log.close_on_exec()
+        self.PIPE = pair = util.pipe()
+        util.set_non_blocking(pair[0])
+        util.set_non_blocking(pair[1])

         # initialize all signals
         [signal.signal(s, self.signal) for s in self.SIGNALS]
diff --git a/gunicorn/glogging.py b/gunicorn/glogging.py
index 7dae434..a6a389f 100644
--- a/gunicorn/glogging.py
+++ b/gunicorn/glogging.py
@@ -161,6 +161,20 @@ def parse_syslog_address(addr):
     return (socktype, (host, port))


+# PEP 446 compatibility
+def _set_inheritable(inheritable):
+    for log in loggers():
+        for handler in log.handlers:
+            if isinstance(handler, logging.FileHandler):
+                handler.acquire()
+                try:
+                    if handler.stream:
+                        fileno = handler.stream.fileno()
+                        util.set_inheritable(fileno, inheritable)
+                finally:
+                    handler.release()
+
+
 class Logger(object):

     LOG_LEVELS = {
@@ -234,6 +248,10 @@ class Logger(object):
                 msg = "Error: log config '%s' not found"
                 raise RuntimeError(msg % cfg.logconfig)

+        # PEP 446 compatibility
+        if sys.version_info < 3.4:
+            _set_inheritable(False)
+
     def critical(self, msg, *args, **kwargs):
         self.error_log.critical(msg, *args, **kwargs)

@@ -357,16 +375,10 @@ class Logger(object):
                     finally:
                         handler.release()

-    def close_on_exec(self):
-        for log in loggers():
-            for handler in log.handlers:
-                if isinstance(handler, logging.FileHandler):
-                    handler.acquire()
-                    try:
-                        if handler.stream:
-                            util.close_on_exec(handler.stream.fileno())
-                    finally:
-                        handler.release()
+
+        # PEP 446 compatibility
+        if sys.version_info < 3.4:
+            _set_inheritable(False)

     def _get_gunicorn_handler(self, log):
         for h in log.handlers:
diff --git a/gunicorn/sock.py b/gunicorn/sock.py
index 7f820eb..0106ca3 100644
--- a/gunicorn/sock.py
+++ b/gunicorn/sock.py
@@ -34,16 +34,36 @@ class BaseSocket(object):
     def __getattr__(self, name):
         return getattr(self.sock, name)

+    # PEP 446 compatibility
+    if sys.version_info < (3, 4):
+        if os.name == 'nt':
+            def accept(self):
+                client, addr = self.sock.accept()
+                util.set_handle_inheritable(client.fileno(), False)
+                return client, addr
+
+            def get_inheritable(self):
+                return util.get_handle_inheritable(self.sock.fileno())
+
+            def set_inheritable(self, inheritable):
+                util.set_handle_inheritable(self.sock.fileno(), inheritable)
+        else:
+            def accept(self):
+                client, addr = self.sock.accept()
+                self.set_inheritable(client.fileno(), False)
+                return client, addr
+
+            def get_inheritable(self):
+                return util.get_inheritable(self.sock.fileno())
+
+            def set_inheritable(self, inheritable):
+                util.set_inheritable(self.sock.fileno(), inheritable)
+
     def set_options(self, sock, bound=False):
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         if not bound:
             self.bind(sock)
         sock.setblocking(0)
-
-        # make sure that the socket can be inherited
-        if hasattr(sock, "set_inheritable"):
-            sock.set_inheritable(True)
-
         sock.listen(self.conf.backlog)
         return sock

diff --git a/gunicorn/systemd.py b/gunicorn/systemd.py
index 10ffb8d..3a741e2 100644
--- a/gunicorn/systemd.py
+++ b/gunicorn/systemd.py
@@ -5,6 +5,8 @@

 import os

+import util
+
 SD_LISTEN_FDS_START = 3


@@ -22,11 +24,8 @@ def listen_fds(unset_environment=True):
     $LISTEN_FDS.

     When $LISTEN_PID matches the current pid, unsets the environment variables
-    unless the ``unset_environment`` flag is ``False``.
-
-    .. note::
-        Unlike the sd_listen_fds C function, this implementation does not set
-        the FD_CLOEXEC flag because the gunicorn arbiter never needs to do this.
+    unless the ``unset_environment`` flag is ``False``, and marks all the parsed
+    file descriptors as non-inheritable.

     .. seealso::
         `<https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html>`_
@@ -42,4 +41,7 @@ def listen_fds(unset_environment=True):
         os.environ.pop('LISTEN_PID', None)
         os.environ.pop('LISTEN_FDS', None)

+    for fileno in range(SD_LISTEN_FDS_START, SD_LISTEN_FDS_START + fds):
+        util.set_inheritable(fileno, False)
+
     return fds
diff --git a/gunicorn/util.py b/gunicorn/util.py
index a011fb8..6cf5810 100644
--- a/gunicorn/util.py
+++ b/gunicorn/util.py
@@ -279,10 +279,30 @@ def get_maxfd():
     return maxfd


-def close_on_exec(fd):
-    flags = fcntl.fcntl(fd, fcntl.F_GETFD)
-    flags |= fcntl.FD_CLOEXEC
-    fcntl.fcntl(fd, fcntl.F_SETFD, flags)
+# PEP 446 compatibility
+if sys.version_info >= (3, 4):
+    pipe = os.pipe
+    get_inheritable = os.get_inheritable
+    set_inheritable = os.set_inheritable
+    if os.name == 'nt':
+        get_handle_inheritable = os.get_handle_inheritable
+        set_handle_inheritable = os.set_handle_inheritable
+else:
+    def pipe():
+        r, w = os.pipe()
+        set_inheritable(r, False)
+        set_inheritable(w, False)
+        return r, w
+
+    def get_inheritable(fd):
+        return bool(fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC)
+
+    def set_inheritable(fd, inheritable):
+        flags = fcntl.fcntl(fd, fcntl.F_GETFD)
+        if inheritable:
+            fcntl.fcntl(fd, fcntl.F_SETFD, flags & ~fcntl.FD_CLOEXEC)
+        else:
+            fcntl.fcntl(fd, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)


 def set_non_blocking(fd):
@@ -565,4 +585,4 @@ def make_fail_app(msg):
         ])
         return [msg]

-    return app
\ No newline at end of file
+    return app
diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py
index 4ceb691..4643122 100644
--- a/gunicorn/workers/base.py
+++ b/gunicorn/workers/base.py
@@ -109,19 +109,16 @@ class Worker(object):
         util.seed()

         # For waking ourselves up
-        self.PIPE = os.pipe()
-        for p in self.PIPE:
-            util.set_non_blocking(p)
-            util.close_on_exec(p)
+        self.PIPE = pair = util.pipe()
+        util.set_non_blocking(pair[0])
+        util.set_non_blocking(pair[1])

         # Prevent fd inheritance
-        [util.close_on_exec(s) for s in self.sockets]
-        util.close_on_exec(self.tmp.fileno())
+        for sock in self.sockets:
+            sock.set_inheritable(False)

         self.wait_fds = self.sockets + [self.PIPE[0]]

-        self.log.close_on_exec()
-
         self.init_signals()

         self.load_wsgi()
diff --git a/gunicorn/workers/sync.py b/gunicorn/workers/sync.py
index 1d2ce2f..2ef6c25 100644
--- a/gunicorn/workers/sync.py
+++ b/gunicorn/workers/sync.py
@@ -26,7 +26,6 @@ class SyncWorker(base.Worker):
     def accept(self, listener):
         client, addr = listener.accept()
         client.setblocking(1)
-        util.close_on_exec(client)
         self.handle(listener, client, addr)

     def wait(self, timeout):
diff --git a/gunicorn/workers/workertmp.py b/gunicorn/workers/workertmp.py
index 36bc97a..4c7de39 100644
--- a/gunicorn/workers/workertmp.py
+++ b/gunicorn/workers/workertmp.py
@@ -5,6 +5,7 @@

 import os
 import platform
+import sys
 import tempfile

 from gunicorn import util
@@ -26,6 +27,10 @@ class WorkerTmp(object):
         util.chown(name, cfg.uid, cfg.gid)
         os.umask(old_umask)

+        # PEP 446 compatibility
+        if sys.version_info < (3, 4):
+            util.set_inheritable(fd, False)
+
         # unlink the file so we don't leak tempory files
         try:
             if not IS_CYGWIN:
Collaborator

tilgovi commented Jul 24, 2016

I addressed all the comments.

The one thing I don't like is that I'm not setting FD_CLOEXEC. I made a note in the docstring about how this deviates from the systemd library calls. I'm thinking maybe we should remove the hack for #978 and sprinkle in PEP 446 compatibility everywhere:

diff --git a/gunicorn/arbiter.py b/gunicorn/arbiter.py
index 7296d2d..9c7cbf6 100644
--- a/gunicorn/arbiter.py
+++ b/gunicorn/arbiter.py
@@ -156,6 +156,10 @@ class Arbiter(object):

             self.LISTENERS = sock.create_sockets(self.cfg, self.log, fds)

+            # Sockets should be inheritable by a new arbiter.
+            for sock in self.LISTENERS:
+                sock.set_inheritable(True)
+
         listeners_str = ",".join([str(l) for l in self.LISTENERS])
         self.log.debug("Arbiter booted")
         self.log.info("Listening at: %s (%s)", listeners_str, self.pid)
@@ -177,12 +181,9 @@ class Arbiter(object):
             [os.close(p) for p in self.PIPE]

         # initialize the pipe
-        self.PIPE = pair = os.pipe()
-        for p in pair:
-            util.set_non_blocking(p)
-            util.close_on_exec(p)
-
-        self.log.close_on_exec()
+        self.PIPE = pair = util.pipe()
+        util.set_non_blocking(pair[0])
+        util.set_non_blocking(pair[1])

         # initialize all signals
         [signal.signal(s, self.signal) for s in self.SIGNALS]
diff --git a/gunicorn/glogging.py b/gunicorn/glogging.py
index 7dae434..a6a389f 100644
--- a/gunicorn/glogging.py
+++ b/gunicorn/glogging.py
@@ -161,6 +161,20 @@ def parse_syslog_address(addr):
     return (socktype, (host, port))


+# PEP 446 compatibility
+def _set_inheritable(inheritable):
+    for log in loggers():
+        for handler in log.handlers:
+            if isinstance(handler, logging.FileHandler):
+                handler.acquire()
+                try:
+                    if handler.stream:
+                        fileno = handler.stream.fileno()
+                        util.set_inheritable(fileno, inheritable)
+                finally:
+                    handler.release()
+
+
 class Logger(object):

     LOG_LEVELS = {
@@ -234,6 +248,10 @@ class Logger(object):
                 msg = "Error: log config '%s' not found"
                 raise RuntimeError(msg % cfg.logconfig)

+        # PEP 446 compatibility
+        if sys.version_info < 3.4:
+            _set_inheritable(False)
+
     def critical(self, msg, *args, **kwargs):
         self.error_log.critical(msg, *args, **kwargs)

@@ -357,16 +375,10 @@ class Logger(object):
                     finally:
                         handler.release()

-    def close_on_exec(self):
-        for log in loggers():
-            for handler in log.handlers:
-                if isinstance(handler, logging.FileHandler):
-                    handler.acquire()
-                    try:
-                        if handler.stream:
-                            util.close_on_exec(handler.stream.fileno())
-                    finally:
-                        handler.release()
+
+        # PEP 446 compatibility
+        if sys.version_info < 3.4:
+            _set_inheritable(False)

     def _get_gunicorn_handler(self, log):
         for h in log.handlers:
diff --git a/gunicorn/sock.py b/gunicorn/sock.py
index 7f820eb..0106ca3 100644
--- a/gunicorn/sock.py
+++ b/gunicorn/sock.py
@@ -34,16 +34,36 @@ class BaseSocket(object):
     def __getattr__(self, name):
         return getattr(self.sock, name)

+    # PEP 446 compatibility
+    if sys.version_info < (3, 4):
+        if os.name == 'nt':
+            def accept(self):
+                client, addr = self.sock.accept()
+                util.set_handle_inheritable(client.fileno(), False)
+                return client, addr
+
+            def get_inheritable(self):
+                return util.get_handle_inheritable(self.sock.fileno())
+
+            def set_inheritable(self, inheritable):
+                util.set_handle_inheritable(self.sock.fileno(), inheritable)
+        else:
+            def accept(self):
+                client, addr = self.sock.accept()
+                self.set_inheritable(client.fileno(), False)
+                return client, addr
+
+            def get_inheritable(self):
+                return util.get_inheritable(self.sock.fileno())
+
+            def set_inheritable(self, inheritable):
+                util.set_inheritable(self.sock.fileno(), inheritable)
+
     def set_options(self, sock, bound=False):
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         if not bound:
             self.bind(sock)
         sock.setblocking(0)
-
-        # make sure that the socket can be inherited
-        if hasattr(sock, "set_inheritable"):
-            sock.set_inheritable(True)
-
         sock.listen(self.conf.backlog)
         return sock

diff --git a/gunicorn/systemd.py b/gunicorn/systemd.py
index 10ffb8d..3a741e2 100644
--- a/gunicorn/systemd.py
+++ b/gunicorn/systemd.py
@@ -5,6 +5,8 @@

 import os

+import util
+
 SD_LISTEN_FDS_START = 3


@@ -22,11 +24,8 @@ def listen_fds(unset_environment=True):
     $LISTEN_FDS.

     When $LISTEN_PID matches the current pid, unsets the environment variables
-    unless the ``unset_environment`` flag is ``False``.
-
-    .. note::
-        Unlike the sd_listen_fds C function, this implementation does not set
-        the FD_CLOEXEC flag because the gunicorn arbiter never needs to do this.
+    unless the ``unset_environment`` flag is ``False``, and marks all the parsed
+    file descriptors as non-inheritable.

     .. seealso::
         `<https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html>`_
@@ -42,4 +41,7 @@ def listen_fds(unset_environment=True):
         os.environ.pop('LISTEN_PID', None)
         os.environ.pop('LISTEN_FDS', None)

+    for fileno in range(SD_LISTEN_FDS_START, SD_LISTEN_FDS_START + fds):
+        util.set_inheritable(fileno, False)
+
     return fds
diff --git a/gunicorn/util.py b/gunicorn/util.py
index a011fb8..6cf5810 100644
--- a/gunicorn/util.py
+++ b/gunicorn/util.py
@@ -279,10 +279,30 @@ def get_maxfd():
     return maxfd


-def close_on_exec(fd):
-    flags = fcntl.fcntl(fd, fcntl.F_GETFD)
-    flags |= fcntl.FD_CLOEXEC
-    fcntl.fcntl(fd, fcntl.F_SETFD, flags)
+# PEP 446 compatibility
+if sys.version_info >= (3, 4):
+    pipe = os.pipe
+    get_inheritable = os.get_inheritable
+    set_inheritable = os.set_inheritable
+    if os.name == 'nt':
+        get_handle_inheritable = os.get_handle_inheritable
+        set_handle_inheritable = os.set_handle_inheritable
+else:
+    def pipe():
+        r, w = os.pipe()
+        set_inheritable(r, False)
+        set_inheritable(w, False)
+        return r, w
+
+    def get_inheritable(fd):
+        return bool(fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC)
+
+    def set_inheritable(fd, inheritable):
+        flags = fcntl.fcntl(fd, fcntl.F_GETFD)
+        if inheritable:
+            fcntl.fcntl(fd, fcntl.F_SETFD, flags & ~fcntl.FD_CLOEXEC)
+        else:
+            fcntl.fcntl(fd, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)


 def set_non_blocking(fd):
@@ -565,4 +585,4 @@ def make_fail_app(msg):
         ])
         return [msg]

-    return app
\ No newline at end of file
+    return app
diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py
index 4ceb691..4643122 100644
--- a/gunicorn/workers/base.py
+++ b/gunicorn/workers/base.py
@@ -109,19 +109,16 @@ class Worker(object):
         util.seed()

         # For waking ourselves up
-        self.PIPE = os.pipe()
-        for p in self.PIPE:
-            util.set_non_blocking(p)
-            util.close_on_exec(p)
+        self.PIPE = pair = util.pipe()
+        util.set_non_blocking(pair[0])
+        util.set_non_blocking(pair[1])

         # Prevent fd inheritance
-        [util.close_on_exec(s) for s in self.sockets]
-        util.close_on_exec(self.tmp.fileno())
+        for sock in self.sockets:
+            sock.set_inheritable(False)

         self.wait_fds = self.sockets + [self.PIPE[0]]

-        self.log.close_on_exec()
-
         self.init_signals()

         self.load_wsgi()
diff --git a/gunicorn/workers/sync.py b/gunicorn/workers/sync.py
index 1d2ce2f..2ef6c25 100644
--- a/gunicorn/workers/sync.py
+++ b/gunicorn/workers/sync.py
@@ -26,7 +26,6 @@ class SyncWorker(base.Worker):
     def accept(self, listener):
         client, addr = listener.accept()
         client.setblocking(1)
-        util.close_on_exec(client)
         self.handle(listener, client, addr)

     def wait(self, timeout):
diff --git a/gunicorn/workers/workertmp.py b/gunicorn/workers/workertmp.py
index 36bc97a..4c7de39 100644
--- a/gunicorn/workers/workertmp.py
+++ b/gunicorn/workers/workertmp.py
@@ -5,6 +5,7 @@

 import os
 import platform
+import sys
 import tempfile

 from gunicorn import util
@@ -26,6 +27,10 @@ class WorkerTmp(object):
         util.chown(name, cfg.uid, cfg.gid)
         os.umask(old_umask)

+        # PEP 446 compatibility
+        if sys.version_info < (3, 4):
+            util.set_inheritable(fd, False)
+
         # unlink the file so we don't leak tempory files
         try:
             if not IS_CYGWIN:
@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Jul 27, 2016

Owner

@tilgovi i would prefer a more explicit api rather than having these version checks inside the code.. Maybe its own module? Otherwise yes let's support this new pep.

Owner

benoitc commented Jul 27, 2016

@tilgovi i would prefer a more explicit api rather than having these version checks inside the code.. Maybe its own module? Otherwise yes let's support this new pep.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Jul 27, 2016

Owner

@tilgovi i still have to test the changes. I plan to do it later today. If ok, let's bump to a major.

Owner

benoitc commented Jul 27, 2016

@tilgovi i still have to test the changes. I plan to do it later today. If ok, let's bump to a major.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Oct 16, 2016

Owner

@tilgovi do we need a major release or a simpler release is OK ?

Owner

benoitc commented Oct 16, 2016

@tilgovi do we need a major release or a simpler release is OK ?

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Dec 20, 2016

Collaborator

@benoitc I fixed the conflicts on the branch.

I'm not sure about making the API more explicit. Are we planning to support py3 < 3.4? If not, then with Gunicorn 20 these checks can just be removed.

Collaborator

tilgovi commented Dec 20, 2016

@benoitc I fixed the conflicts on the branch.

I'm not sure about making the API more explicit. Are we planning to support py3 < 3.4? If not, then with Gunicorn 20 these checks can just be removed.

@berkerpeksag

This comment has been minimized.

Show comment
Hide comment
@berkerpeksag

berkerpeksag Dec 22, 2016

Collaborator

+1 for supporting 3.4 and later. September '17 is the EOL date for 3.3: https://docs.python.org/devguide/index.html#status-of-python-branches

Collaborator

berkerpeksag commented Dec 22, 2016

+1 for supporting 3.4 and later. September '17 is the EOL date for 3.3: https://docs.python.org/devguide/index.html#status-of-python-branches

Refactor socket activation and fd inheritance
Track the use of systemd socket activation and gunicorn socket inheritance
in the arbiter. Unify the logic of creating gunicorn sockets from each of
these sources to always use the socket name to determine the type rather
than checking the configured addresses. The configured addresses are only
used when there is no inheritance from systemd or a parent arbiter.

Fix #1298
@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Dec 27, 2016

Collaborator

I'd love to merge this if there are no objections. I will add PEP 446 compatibility in a separate PR.

Collaborator

tilgovi commented Dec 27, 2016

I'd love to merge this if there are no objections. I will add PEP 446 compatibility in a separate PR.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Dec 27, 2016

Owner

@tilgovi go ahead :)

Owner

benoitc commented Dec 27, 2016

@tilgovi go ahead :)

@benoitc benoitc self-requested a review Dec 27, 2016

@tilgovi tilgovi merged commit 0be7996 into master Dec 27, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@tilgovi tilgovi deleted the fix/1298-2 branch Dec 27, 2016

fofanov pushed a commit to fofanov/gunicorn that referenced this pull request Mar 16, 2018

Refactor socket activation and fd inheritance (#1310)
Track the use of systemd socket activation and gunicorn socket inheritance
in the arbiter. Unify the logic of creating gunicorn sockets from each of
these sources to always use the socket name to determine the type rather
than checking the configured addresses. The configured addresses are only
used when there is no inheritance from systemd or a parent arbiter.

Fix #1298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment