[jsonrpc] Workaround file descriptor lifetime issue

We should solve this in the JSONRPCConnection itself, but in the
meantime workaround by keeping pipes alive until the connection has
closed.

Author: benlangmuir

Sources/LSPTestSupport/TestJSONRPCConnection.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -18,7 +18,7 @@ import XCTest
 // Workaround ambiguity with Foundation.
 public typealias Notification = LanguageServerProtocol.Notification
 
-public struct TestJSONRPCConnection {
+public final class TestJSONRPCConnection {
   public let clientToServer: Pipe = Pipe()
   public let serverToClient: Pipe = Pipe()
   public let client: TestClient
@@ -27,11 +27,6 @@ public struct TestJSONRPCConnection {
   public let serverConnection: JSONRPCConnection
 
   public init() {
-    // FIXME: DispatchIO doesn't like when the Pipes close behind its back even after the tests
-    // finish. Until we fix the lifetime, leak.
-    _ = Unmanaged.passRetained(clientToServer)
-    _ = Unmanaged.passRetained(serverToClient)
-
     clientConnection = JSONRPCConnection(
       protocol: testMessageRegistry,
       inFD: serverToClient.fileHandleForReading.fileDescriptor,
@@ -47,8 +42,16 @@ public struct TestJSONRPCConnection {
     client = TestClient(server: clientConnection)
     server = TestServer(client: serverConnection)
 
-    clientConnection.start(receiveHandler: client)
-    serverConnection.start(receiveHandler: server)
+    clientConnection.start(receiveHandler: client) {
+      // FIXME: keep the pipes alive until we close the connection. This
+      // should be fixed systemically.
+      withExtendedLifetime(self) {}
+    }
+    serverConnection.start(receiveHandler: server) {
+      // FIXME: keep the pipes alive until we close the connection. This
+      // should be fixed systemically.
+      withExtendedLifetime(self) {}
+    }
   }
 
   public func close() {

Sources/SKCore/BuildServerBuildSystem.swift

@@ -254,7 +254,11 @@ private func makeJSONRPCBuildServer(client: MessageHandler, serverPath: Absolute
     outFD: clientToServer.fileHandleForWriting.fileDescriptor
   )
 
-  connection.start(receiveHandler: client)
+  connection.start(receiveHandler: client) {
+    // FIXME: keep the pipes alive until we close the connection. This
+    // should be fixed systemically.
+    withExtendedLifetime((clientToServer, serverToClient)) {}
+  }
   let process = Foundation.Process()
 
   if #available(OSX 10.13, *) {

Sources/SKTestSupport/TestServer.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -63,11 +63,6 @@ public struct TestSourceKitServer {
         let clientToServer: Pipe = Pipe()
         let serverToClient: Pipe = Pipe()
 
-        // FIXME: DispatchIO doesn't like when the Pipes close behind its back even after the tests
-        // finish. Until we fix the lifetime, leak.
-        _ = Unmanaged.passRetained(clientToServer)
-        _ = Unmanaged.passRetained(serverToClient)
-
         let clientConnection = JSONRPCConnection(
           protocol: MessageRegistry.lspProtocol,
           inFD: serverToClient.fileHandleForReading.fileDescriptor,
@@ -84,8 +79,16 @@ public struct TestSourceKitServer {
           serverConnection.close()
         })
 
-        clientConnection.start(receiveHandler: client)
-        serverConnection.start(receiveHandler: server!)
+        clientConnection.start(receiveHandler: client) {
+          // FIXME: keep the pipes alive until we close the connection. This
+          // should be fixed systemically.
+          withExtendedLifetime((clientToServer, serverToClient)) {}
+        }
+        serverConnection.start(receiveHandler: server!) {
+          // FIXME: keep the pipes alive until we close the connection. This
+          // should be fixed systemically.
+          withExtendedLifetime((clientToServer, serverToClient)) {}
+        }
 
         connImpl = .jsonrpc(
           clientToServer: clientToServer,

Sources/SourceKitLSP/Clang/ClangLanguageServer.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -247,7 +247,11 @@ func makeJSONRPCClangServer(
     clang: toolchain.clang)
 
   connectionToClient.start(handler: client)
-  connection.start(receiveHandler: client)
+  connection.start(receiveHandler: client) {
+    // FIXME: keep the pipes alive until we close the connection. This
+    // should be fixed systemically.
+    withExtendedLifetime((clientToServer, serverToClient)) {}
+  }
 
   let process = Foundation.Process()
 

Tests/LanguageServerProtocolJSONRPCTests/ConnectionTests.swift

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -237,6 +237,10 @@ class ConnectionTests: XCTestCase {
       conn.start(receiveHandler: DummyHandler(), closeHandler: {
         // We get an error from XCTest if this is fulfilled more than once.
         expectation.fulfill()
+
+        // FIXME: keep the pipes alive until we close the connection. This
+        // should be fixed systemically.
+        withExtendedLifetime((to, from)) {}
       })
 
       to.fileHandleForWriting.closeFile()