Patchwork [1,of,2] commandserver: do not handle EINTR

login
register
mail settings
Submitter Jun Wu
Date July 16, 2017, 12:03 p.m.
Message ID <e9e6e076ffef9d11fa1f.1500206586@x1c>
Download mbox | patch
Permalink /patch/22437/
State Accepted
Headers show

Comments

Jun Wu - July 16, 2017, 12:03 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1500205043 25200
#      Sun Jul 16 04:37:23 2017 -0700
# Node ID e9e6e076ffef9d11fa1f46201e85a3c4570a521a
# Parent  389536aff376d32d38f13305021c127245d4126a
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r e9e6e076ffef
commandserver: do not handle EINTR

EINTR was handled by selectors2 library so we don't need to do it again.
Yuya Nishihara - July 16, 2017, 2:15 p.m.
On Sun, 16 Jul 2017 05:03:06 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1500205043 25200
> #      Sun Jul 16 04:37:23 2017 -0700
> # Node ID e9e6e076ffef9d11fa1f46201e85a3c4570a521a
> # Parent  389536aff376d32d38f13305021c127245d4126a
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r e9e6e076ffef
> commandserver: do not handle EINTR
> 
> EINTR was handled by selectors2 library so we don't need to do it again.
> 
> diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
> --- a/mercurial/commandserver.py
> +++ b/mercurial/commandserver.py
> @@ -12,5 +12,4 @@ import gc
>  import os
>  import random
> -import select
>  import signal
>  import socket
> @@ -489,17 +488,11 @@ class unixforkingservice(object):
>                  self._unlinksocket()
>                  exiting = True
> -            try:
> -                ready = selector.select(timeout=h.pollinterval)
> -                if not ready:
> -                    # only exit if we completed all queued requests
> -                    if exiting:
> -                        break
> -                    continue
> -                conn, _addr = self._sock.accept()
> -            except (select.error, socket.error) as inst:
> -                if inst.args[0] == errno.EINTR:
> -                    continue
> -                raise
> -
> +            ready = selector.select(timeout=h.pollinterval)
> +            if not ready:
> +                # only exit if we completed all queued requests
> +                if exiting:
> +                    break
> +                continue
> +            conn, _addr = self._sock.accept()

IIUC, accept() could raise EINTR, though that wouldn't occur in practice.
Jun Wu - July 16, 2017, 6:15 p.m.
Excerpts from Yuya Nishihara's message of 2017-07-16 23:15:05 +0900:
> IIUC, accept() could raise EINTR, though that wouldn't occur in practice.

Good catch. I checked CPython 2.7 source code and (sadly) confirmed it
misses EINTR handling here.

Patch

diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
--- a/mercurial/commandserver.py
+++ b/mercurial/commandserver.py
@@ -12,5 +12,4 @@  import gc
 import os
 import random
-import select
 import signal
 import socket
@@ -489,17 +488,11 @@  class unixforkingservice(object):
                 self._unlinksocket()
                 exiting = True
-            try:
-                ready = selector.select(timeout=h.pollinterval)
-                if not ready:
-                    # only exit if we completed all queued requests
-                    if exiting:
-                        break
-                    continue
-                conn, _addr = self._sock.accept()
-            except (select.error, socket.error) as inst:
-                if inst.args[0] == errno.EINTR:
-                    continue
-                raise
-
+            ready = selector.select(timeout=h.pollinterval)
+            if not ready:
+                # only exit if we completed all queued requests
+                if exiting:
+                    break
+                continue
+            conn, _addr = self._sock.accept()
             pid = os.fork()
             if pid: