On Windows, we need to use two named pipes, otherwise things get really complicated fast

Source/Atomic/IPC/IPC.cpp

@@ -41,10 +41,14 @@ bool IPC::InitWorker(unsigned id, IPCHandle fd1, IPCHandle fd2)
 #ifndef ATOMIC_PLATFORM_WINDOWS
     // close server fd
     close(fd1);
+	worker_ = new IPCWorker(context_, fd2, id);
+#else
+	worker_ = new IPCWorker(context_, fd1, fd2, id);
 #endif
 
-    worker_ = new IPCWorker(context_, fd2, id);
-    worker_->Run();
+    
+    
+	worker_->Run();
 
     SendEventToBroker(E_IPCWORKERSTART);
 

Source/Atomic/IPC/IPCBroker.cpp

@@ -66,9 +66,9 @@ bool IPCBroker::SpawnWorker(const String& command, const Vector<String>& args, c
 {
     Vector<String> pargs;
 
-    otherProcess_ = new IPCProcess(context_, pp_.fd1(), pp_.fd2());
+    otherProcess_ = new IPCProcess(context_, pp_.clientRead(), pp_.clientWrite());
 
-    transport_.OpenServer(pp_.fd1());
+    transport_.OpenServer(pp_.serverRead(), pp_.serverWrite());
 
     // copy args
     for (unsigned i = 0; i < args.Size(); i++)
@@ -77,7 +77,7 @@ bool IPCBroker::SpawnWorker(const String& command, const Vector<String>& args, c
 #ifdef ATOMIC_PLATFORM_WINDOWS
 
     wchar_t pipe_num[10];
-    _i64tow_s(reinterpret_cast<__int64>(pp_.fd2()), pipe_num, sizeof(pipe_num)/sizeof(pipe_num[0]), 10);
+    _i64tow_s(reinterpret_cast<__int64>(pp_.clientWrite()), pipe_num, sizeof(pipe_num)/sizeof(pipe_num[0]), 10);
     
     String cpipe;
     cpipe.SetUTF8FromWChar(pipe_num);
@@ -85,7 +85,7 @@ bool IPCBroker::SpawnWorker(const String& command, const Vector<String>& args, c
     String ipc_client = "--ipc-client=" + cpipe;
     pargs.Push(ipc_client);
 
-    _i64tow_s(reinterpret_cast<__int64>(pp_.fd1()), pipe_num, sizeof(pipe_num)/sizeof(pipe_num[0]), 10);
+    _i64tow_s(reinterpret_cast<__int64>(pp_.clientRead()), pipe_num, sizeof(pipe_num)/sizeof(pipe_num[0]), 10);
 
     String spipe;
     spipe.SetUTF8FromWChar(pipe_num);

Source/Atomic/IPC/IPCWindows.cpp

@@ -17,48 +17,34 @@ namespace Atomic
     static const int kPipeBufferSz = 4 * 1024;
     static LONG g_pipe_seq = 0;
 
-    bool checkIntegritySupport()
-    {
-        OSVERSIONINFO osvi;
-
-        ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
-        osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
-        GetVersionEx(&osvi);
-
-        return osvi.dwMajorVersion > 5;
-    }
-
-    HANDLE PipePair::OpenPipeServer(const wchar_t* name, bool low_integrity) {
-        SECURITY_ATTRIBUTES sa = { 0 };
-        SECURITY_ATTRIBUTES *psa = 0;
-
-        static const bool is_integrity_supported = false;// checkIntegritySupport();
-
-        if (is_integrity_supported && low_integrity) {
-            psa = &sa;
-            sa.nLength = sizeof(SECURITY_ATTRIBUTES);
-            sa.bInheritHandle = TRUE;
-            if (!ConvertStringSecurityDescriptorToSecurityDescriptor(
-                TEXT("S:(ML;;NWNR;;;LW)"), SDDL_REVISION_1, &sa.lpSecurityDescriptor, NULL))
-                return INVALID_HANDLE_VALUE;
-        }
-
+    HANDLE PipePair::OpenPipeServer(const wchar_t* name, bool read) 
+	{
         IPCWString pipename(kPipePrefix);
         pipename.append(name);
-        return ::CreateNamedPipeW(pipename.c_str(), PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+
+		DWORD openMode = read ? PIPE_ACCESS_INBOUND : PIPE_ACCESS_OUTBOUND;
+
+        return ::CreateNamedPipeW(pipename.c_str(), openMode,
             PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
-            1, kPipeBufferSz, kPipeBufferSz, 200, psa);
+            1, kPipeBufferSz, kPipeBufferSz, 200, NULL);
     }
 
-    HANDLE PipePair::OpenPipeClient(const wchar_t* name, bool inherit, bool impersonate) {
+    HANDLE PipePair::OpenPipeClient(const wchar_t* name, bool read) 
+	{
         IPCWString pipename(kPipePrefix);
         pipename.append(name);
 
-        SECURITY_ATTRIBUTES sa = { sizeof(sa), NULL, inherit ? TRUE : FALSE };
+		SECURITY_ATTRIBUTES sa;
+		sa.bInheritHandle = TRUE;
+		sa.lpSecurityDescriptor = NULL;
+		sa.nLength = sizeof(SECURITY_ATTRIBUTES);
+
+		DWORD accessMode = read ? GENERIC_READ : GENERIC_WRITE;
+
         for (;;) {
-            DWORD attributes = impersonate ? 0 : SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION;
-            HANDLE pipe = ::CreateFileW(pipename.c_str(), GENERIC_READ | GENERIC_WRITE, 0, &sa,
-                OPEN_EXISTING, attributes, NULL);
+           
+			HANDLE pipe = ::CreateFileW(pipename.c_str(), accessMode, 0, &sa,
+                OPEN_EXISTING, 0, NULL);
 
             if (INVALID_HANDLE_VALUE == pipe) {
                 if (ERROR_PIPE_BUSY != ::GetLastError()) {
@@ -74,92 +60,102 @@ namespace Atomic
         }
     }
 
-    PipePair::PipePair(bool inherit_fd2) :
-        srv_(INVALID_IPCHANDLE_VALUE),
-        cln_(INVALID_IPCHANDLE_VALUE)
+    PipePair::PipePair() :
+        srvRead_(INVALID_IPCHANDLE_VALUE),
+		srvWrite_(INVALID_IPCHANDLE_VALUE),
+		clnRead_(INVALID_IPCHANDLE_VALUE),
+		clnWrite_(INVALID_IPCHANDLE_VALUE)        
     {
         // Come up with a reasonable unique name.
-        const wchar_t kPipePattern[] = L"ko.%x.%x.%x";
-        
-        wchar_t name[8 * 3 + sizeof(kPipePattern)];
-        
-        ::wsprintfW(name, kPipePattern, ::GetCurrentProcessId(), ::GetTickCount(),
+        const wchar_t kPipePattern[] = L"ko.%x.%x.%x";        
+
+        wchar_t serverReadName[8 * 3 + sizeof(kPipePattern)];        
+        ::wsprintfW(serverReadName, kPipePattern, ::GetCurrentProcessId(), ::GetTickCount(),
             ::InterlockedIncrement(&g_pipe_seq));
-        
-        HANDLE server = OpenPipeServer(name);
-        
-        if (INVALID_HANDLE_VALUE == server) 
-        {
-            return;
-        }
-        
+
+		wchar_t serverWriteName[8 * 3 + sizeof(kPipePattern)];
+		::wsprintfW(serverWriteName, kPipePattern, ::GetCurrentProcessId(), ::GetTickCount() + 1,
+			::InterlockedIncrement(&g_pipe_seq));
+
+        srvRead_ = OpenPipeServer(serverReadName, true);
+		srvWrite_ = OpenPipeServer(serverWriteName, false);
+
+
         // Don't allow client impersonation.
-        HANDLE client = OpenPipeClient(name, inherit_fd2, false);
+        clnRead_ = OpenPipeClient(serverWriteName, true);
+		clnWrite_ = OpenPipeClient(serverReadName, false);
 
+		/*
         if (INVALID_HANDLE_VALUE == client) 
         {
             ::CloseHandle(server);
             return;
         }
-        if (!::ConnectNamedPipe(server, NULL)) 
+		*/
+        
+		if (!::ConnectNamedPipe(srvRead_, NULL))
         {
             if (ERROR_PIPE_CONNECTED != ::GetLastError()) 
             {
-                ::CloseHandle(server);
-                ::CloseHandle(client);
+               // ::CloseHandle(server);
+                //::CloseHandle(client);
                 return;
             }
         }
 
-        srv_ = server;
-        cln_ = client;
+		if (!::ConnectNamedPipe(srvWrite_, NULL))
+		{
+			if (ERROR_PIPE_CONNECTED != ::GetLastError())
+			{
+				// ::CloseHandle(server);
+				//::CloseHandle(client);
+				return;
+			}
+		}
 
     }
 
-    PipeWin::PipeWin() : pipe_(INVALID_IPCHANDLE_VALUE)
+    PipeWin::PipeWin() : pipeRead_(INVALID_IPCHANDLE_VALUE), pipeWrite_(INVALID_IPCHANDLE_VALUE)
     {
 
     }
 
     PipeWin::~PipeWin()
     {
-        if (pipe_ != INVALID_HANDLE_VALUE) 
+        if (pipeRead_ != INVALID_HANDLE_VALUE) 
         {
-            ::DisconnectNamedPipe(pipe_);  // $$$ disconect is valid on the server side.
-            ::CloseHandle(pipe_);
+            ::DisconnectNamedPipe(pipeRead_);  // $$$ disconect is valid on the server side.
+            ::CloseHandle(pipeRead_);
         }
+
+		if (pipeWrite_ != INVALID_HANDLE_VALUE)
+		{
+			::DisconnectNamedPipe(pipeWrite_);  // $$$ disconect is valid on the server side.
+			::CloseHandle(pipeWrite_);
+		}
+
     }
 
-    bool PipeWin::OpenClient(IPCHandle pipe)
+    bool PipeWin::OpenClient(IPCHandle pipeRead, IPCHandle pipeWrite)
     {
-        pipe_ = pipe;
+        pipeRead_ = pipeRead;
+		pipeWrite_ = pipeWrite;
         return true;
 
     }
 
-    bool PipeWin::OpenServer(IPCHandle pipe, bool connect)
+    bool PipeWin::OpenServer(IPCHandle pipeRead, IPCHandle pipeWrite)
     {
-        pipe_ = pipe;
-
-        if (connect) 
-        {
-            if (!::ConnectNamedPipe(pipe, NULL)) 
-            {
-                if (ERROR_PIPE_CONNECTED != ::GetLastError()) 
-                {
-                    return false;
-                }
-            }
-        }
-
-        return true;
-    }
+		pipeRead_ = pipeRead;
+		pipeWrite_ = pipeWrite;
+		return true;
+	}
 
 
     bool PipeWin::Write(const void* buf, size_t sz)
     {
         DWORD written = 0;
-        if (TRUE == ::WriteFile(pipe_, buf, sz, &written, NULL))
+        if (TRUE == ::WriteFile(pipeWrite_, buf, sz, &written, NULL))
             return true;
 
         return false;
@@ -167,7 +163,7 @@ namespace Atomic
 
     bool PipeWin::Read(void* buf, size_t* sz)
     {
-        if (TRUE == ::ReadFile(pipe_, buf, *sz, reinterpret_cast<DWORD*>(sz), NULL))
+        if (TRUE == ::ReadFile(pipeRead_, buf, *sz, reinterpret_cast<DWORD*>(sz), NULL))
         {
             return true;
         }
@@ -192,10 +188,10 @@ namespace Atomic
     }
 
 
-    IPCProcess::IPCProcess(Context* context, IPCHandle fd1, IPCHandle fd2, IPCHandle pid) : Object(context),
+    IPCProcess::IPCProcess(Context* context, IPCHandle clientRead, IPCHandle clientWrite, IPCHandle pid) : Object(context),
         pid_(pid),
-        fd1_(fd1),
-        fd2_(fd2)
+        clientRead_(clientRead),
+        clientWrite_(clientWrite)
     {
     }
 
@@ -233,9 +229,6 @@ namespace Atomic
         pid_ = pi.hProcess;
         ::CloseHandle(pi.hThread);
         
-        // The client side of the pipe has been already duplicated into the worker process.
-        ::CloseHandle(fd2_);
-
         return true;
     }
 

Source/Atomic/IPC/IPCWindows.h

@@ -12,16 +12,23 @@ namespace Atomic
 class PipePair {
 
 public:
-    PipePair(bool inherit_fd2 = true);
-    IPCHandle fd1() const { return srv_; }
-    IPCHandle fd2() const { return cln_; }
+    PipePair();
 
-    static IPCHandle OpenPipeServer(const wchar_t* name, bool low_integrity = true);
-    static IPCHandle OpenPipeClient(const wchar_t* name, bool inherit, bool impersonate);
+    IPCHandle serverRead() const { return srvRead_; }
+    IPCHandle serverWrite() const { return srvWrite_; }
+
+	IPCHandle clientRead() const { return clnRead_; }
+	IPCHandle clientWrite() const { return clnWrite_; }
+
+    static IPCHandle OpenPipeServer(const wchar_t* name, bool read);
+    static IPCHandle OpenPipeClient(const wchar_t* name, bool read);
 
 private:
-    IPCHandle srv_;
-    IPCHandle cln_;
+    IPCHandle srvRead_;
+	IPCHandle srvWrite_;
+
+	IPCHandle clnRead_;
+	IPCHandle clnWrite_;
 };
 
 class PipeWin {
@@ -29,16 +36,17 @@ public:
     PipeWin();
     ~PipeWin();
 
-    bool OpenClient(IPCHandle pipe);
-    bool OpenServer(IPCHandle pipe, bool connect = false);
+    bool OpenClient(IPCHandle pipeRead, IPCHandle pipeWrite);
+    bool OpenServer(IPCHandle pipeRead, IPCHandle pipeWrite);
 
     bool Write(const void* buf, size_t sz);
     bool Read(void* buf, size_t* sz);
 
-    bool IsConnected() const { return pipe_ != INVALID_IPCHANDLE_VALUE; }
+    bool IsConnected() const { return pipeRead_ != INVALID_IPCHANDLE_VALUE && pipeWrite_ != INVALID_IPCHANDLE_VALUE; }
 
 private:
-    IPCHandle pipe_;
+    IPCHandle pipeRead_;
+	IPCHandle pipeWrite_;
 };
 
 
@@ -62,22 +70,22 @@ class IPCProcess : public Object
 
     public:
 
-    IPCProcess(Context* context, IPCHandle fd1, IPCHandle fd2, IPCHandle pid = INVALID_IPCHANDLE_VALUE);
+    IPCProcess(Context* context, IPCHandle clientRead, IPCHandle clientWrite, IPCHandle pid = INVALID_IPCHANDLE_VALUE);
 
     virtual ~IPCProcess();
 
     bool IsRunning();
 
-    IPCHandle fd1() const { return fd1_; }
-    IPCHandle fd2() const { return fd2_; }
+    IPCHandle clientRead() const { return clientRead_; }
+    IPCHandle clientWrite() const { return clientWrite_; }
 
     bool Launch(const String& command, const Vector<String>& args, const String& initialDirectory);
 
 private:
 
     IPCHandle pid_;
-    IPCHandle fd1_;
-    IPCHandle fd2_;
+    IPCHandle clientRead_;
+    IPCHandle clientWrite_;
 };
 
 }

Source/Atomic/IPC/IPCWorker.cpp

@@ -15,25 +15,45 @@
 namespace Atomic
 {
 
+IPCWorker::IPCWorker(Context* context, IPCHandle clientRead, IPCHandle clientWrite, unsigned id) : IPCChannel(context, id)
+{
+#ifndef ATOMIC_PLATFORM_WINDOWS
+	assert(0); // wrong constructor
+#else
+	otherProcess_ = new IPCProcess(context_, clientRead, clientWrite, INVALID_IPCHANDLE_VALUE);
+
+	if (!transport_.OpenClient(clientRead, clientWrite))
+	{
+		LOGERRORF("Unable to open IPC transport clientRead = %i", clientRead);
+		shouldRun_ = false;
+		return;
+	}
+
+	LOGERRORF("Opened IPC transport fd = %i", clientRead);
+
+#endif
+
+}
+
 IPCWorker::IPCWorker(Context* context, IPCHandle fd, unsigned id) : IPCChannel(context, id),
-    fd_(fd)
+    clientRead_(fd),
+	clientWrite_(fd)
 {
 
 #ifdef ATOMIC_PLATFORM_WINDOWS
-    otherProcess_ = new IPCProcess(context_, INVALID_IPCHANDLE_VALUE, fd_, INVALID_IPCHANDLE_VALUE);
+	assert(0); // wrong constructor
 #else
-    otherProcess_ = new IPCProcess(context_, -1, fd, getppid());
-#endif
+    otherProcess_ = new IPCProcess(context_, -1, clientRead_, getppid());
 
-    if (!transport_.OpenClient(fd_))
+    if (!transport_.OpenClient(clientRead_))
     {
-        LOGERRORF("Unable to open IPC transport fd = %i", fd_);
+        LOGERRORF("Unable to open IPC transport fd = %i", clientRead_);
         shouldRun_ = false;
         return;
     }
 
-    LOGERRORF("Opened IPC transport fd = %i", fd_);
-
+    LOGERRORF("Opened IPC transport fd = %i", clientRead_);
+#endif
 }
 
 IPCWorker::~IPCWorker()

Source/Atomic/IPC/IPCWorker.h

@@ -13,9 +13,13 @@ class IPCWorker : public IPCChannel
     OBJECT(IPCWorker);
 
 public:
-    /// Construct.
-    IPCWorker(Context* context, IPCHandle fd, unsigned id);
-    /// Destruct.
+    /// POSIX Constructor   
+	IPCWorker(Context* context, IPCHandle fd, unsigned id);
+
+	// Windows Constructor, two named pipes are used
+	IPCWorker(Context* context, IPCHandle clientRead, IPCHandle clientWrite, unsigned id);
+    
+	/// Destruct.
     virtual ~IPCWorker();
 
     void ThreadFunction();
@@ -24,7 +28,9 @@ public:
 
 private:
 
-    IPCHandle fd_;
+	// on unix will be the same
+    IPCHandle clientRead_;
+	IPCHandle clientWrite_;
 
 };
 

Source/AtomicEditorWork/PlayerMode/AEPlayerMode.cpp

@@ -78,6 +78,7 @@ void PlayerMode::ProcessArguments() {
                     if (argument.StartsWith("--ipc-server="))
                     {
 #ifdef ATOMIC_PLATFORM_WINDOWS
+						// clientRead
                         WString wipc(ipc[1]);
                         HANDLE pipe = reinterpret_cast<HANDLE>(_wtoi64(wipc.CString()));
                         fd_[0] = pipe;
@@ -89,6 +90,7 @@ void PlayerMode::ProcessArguments() {
                     else
                     {
 #ifdef ATOMIC_PLATFORM_WINDOWS
+						// clientWrite
                         WString wipc(ipc[1]);
                         HANDLE pipe = reinterpret_cast<HANDLE>(_wtoi64(wipc.CString()));
                         fd_[1] = pipe;
@@ -105,26 +107,11 @@ void PlayerMode::ProcessArguments() {
         }
     }
 
-#ifdef ATOMIC_PLATFORM_WINDOWS
-    if (fd_[0] != INVALID_IPCHANDLE_VALUE)
-    {
-        //::CloseHandle(fd_[0]);
-        fd_[0] = INVALID_IPCHANDLE_VALUE;
-    }
-
-    if (fd_[1] != INVALID_IPCHANDLE_VALUE)
-    {
-        IPC* ipc = new IPC(context_);
-        context_->RegisterSubsystem(ipc);
-        //ipc->InitWorker(fd_[0], fd_[1]);
-    }
-#else
     if (id > 0 && fd_[0] != INVALID_IPCHANDLE_VALUE && fd_[1] != INVALID_IPCHANDLE_VALUE)
     {        
         SubscribeToEvent(E_IPCINITIALIZE, HANDLER(PlayerMode, HandleIPCInitialize));
         ipc_->InitWorker((unsigned) id, fd_[0], fd_[1]);
     }
-#endif
 
 }