Merge lp:~brandontschaefer/libertine/rlist-race into lp:libertine

[Brandon Schaefer]

Fix a race when two sockets in the read list from select were pairs and the first one closed its connection.

How this happens:
1) We get two sockets in the read list from select
2) Both those sockets are pairs (ie. read from 1 send to 2)
3) The first socket has its connection closed (ie. recv gave us 0 data)
4) Then when the second socket reads data to send to the first socket that was closed
   it is not found since it was just cleaned up.

The current fix:
Check if our socket is not in the session sockets or in the pairs. If thats true we stopped watching this socket so skip to the next read!

[Libertine CI Bot (libertine-ci-bot)]

PASSED: Continuous integration, rev:287
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/100/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/286
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/236
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/236
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/236
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/236
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/236
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/236
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/217/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/288
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/272
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/272
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/272
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/265/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/265/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/265/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/265/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/265/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/265
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/265/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/100/rebuild

review: Approve (continuous-integration)

[Christopher Townsend (townsend)]

Yes, race is indeed fixed. Thanks!

review: Approve

=== modified file 'python/libertine/Libertine.py'
--- python/libertine/Libertine.py	2016-08-11 17:07:44 +0000
+++ python/libertine/Libertine.py	2016-08-16 18:18:12 +0000
@@ -444,7 +444,6 @@
 class Socket(object):
     """
     A simple socket wrapper class. This will wrap a socket
-    This is used for RAII and to take ownership of the socket
 
     :param socket: A python socket to be wrapped
     """
@@ -454,10 +453,6 @@
 
         self.socket = sock
 
-    def __del__(self):
-        self.socket.shutdown(SHUT_RDWR)
-        self.socket.close()
-
     """
     Implement equality checking for other instances of this class or ints only.
 
@@ -485,6 +480,7 @@
 class SessionSocket(Socket):
     """
     Creates a AF_UNIX socket from a path to be used as the session socket.
+    This is used for RAII and to take ownership of the socket
 
     :param path: The path the socket will be binded with
     """
@@ -507,7 +503,8 @@
                 self.session_path = session_path
 
     def __del__(self):
-        super().__del__()
+        self.socket.shutdown(SHUT_RDWR)
+        self.socket.close()
         os.remove(self.session_path)
 
 
@@ -600,6 +597,11 @@
                 if sock.fileno() == -1:
                     continue
 
+                # Its possible to have multiple socket reads that are pairs. If this happens and we remove a pair we
+                # need to ignore the other pair since it no longer has a complete pair
+                if sock.fileno() not in self.host_session_socket_path_map and sock.fileno() not in self.socket_pairs:
+                    continue
+
                 if sock.fileno() in self.host_session_socket_path_map:
                     self.accept_new_connection(self.host_session_socket_path_map[sock.fileno()], sock)