You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by GitBox <gi...@apache.org> on 2022/09/26 03:46:43 UTC

[GitHub] [thrift] kainjow commented on a diff in pull request #2670: Thrift 4547: Add Swift cross tests

kainjow commented on code in PR #2670:
URL: https://github.com/apache/thrift/pull/2670#discussion_r979538131


##########
lib/swift/Sources/TSocketServer.swift:
##########
@@ -96,34 +96,29 @@ open class TSocketServer<InProtocol: TProtocol, OutProtocol: TProtocol, Processo
 
     // throw away our socket
     CFSocketInvalidate(sock)
-
-    // register for notifications of accepted incoming connections
-    _ = NotificationCenter.default.addObserver(forName: .NSFileHandleConnectionAccepted,
-                                               object: nil, queue: nil) {
-                                                [weak self] notification in
-                                                guard let strongSelf = self else { return }
-                                                guard let clientSocket = notification.userInfo?[NSFileHandleNotificationFileHandleItem] as? FileHandle else { return }
-                                                strongSelf.connectionAccepted(clientSocket)
-    }
-
-    // tell socket to listen
-    socketFileHandle.acceptConnectionInBackgroundAndNotify()
-
+    
     print("TSocketServer: Listening on TCP port \(port)")
+    
+    // tell socket to listen
+    acceptConnectionInBackgroundAndNotify(handle: socketFileHandle)
   }
-
-  deinit {
-    NotificationCenter.default.removeObserver(self)
+  
+  open func acceptConnectionInBackgroundAndNotify(handle: FileHandle) {
+    DispatchQueue(label: "ConnectionAcceptQueue").async {

Review Comment:
   should this label match the form of the label used above? also is `Queue` in the name redundant? `TSocketServer.processing`
   maybe `TSocketServer.connectionAccept`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org