From 8121e5408e06a735361c640fd7338e167f22365f Mon Sep 17 00:00:00 2001 From: CBA Date: Fri, 24 Mar 2017 15:05:41 +0100 Subject: [PATCH 1/8] Add include directory of installation of Microsoft R Open 3.3.2. --- RserveVisualStudio/Rserve/Rserve.vcxproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RserveVisualStudio/Rserve/Rserve.vcxproj b/RserveVisualStudio/Rserve/Rserve.vcxproj index 57eb53a..56c72e8 100644 --- a/RserveVisualStudio/Rserve/Rserve.vcxproj +++ b/RserveVisualStudio/Rserve/Rserve.vcxproj @@ -101,7 +101,7 @@ Level3 Disabled WIN32;_DEBUG;_CONSOLE;_LIB;Win32;_R_;RSERV_DEBUG;NO_CONFIG_H;_SECURE;%(PreprocessorDefinitions) - $(ProjectDir);$(ProjectDir)..\..\Rserve\src;$(ProjectDir)..\..\Rserve\src\include;C:\Program Files\Microsoft\R Server\R_SERVER\include;C:\Program Files\Microsoft\R Server\R_SERVER\include\R_ext + $(ProjectDir);$(ProjectDir)..\..\Rserve\src;$(ProjectDir)..\..\Rserve\src\include;C:\Program Files\Microsoft\R Server\R_SERVER\include;C:\Program Files\Microsoft\R Server\R_SERVER\include\R_ext;c:\Program Files\Microsoft\MRO-3.3.2\include\ MultiThreadedDebug From a7a0d11c325af966c07d1ec2f78ae5bf50fade44 Mon Sep 17 00:00:00 2001 From: CBA Date: Fri, 24 Mar 2017 17:24:47 +0100 Subject: [PATCH 2/8] Fix R package built: disable call of configure.win that threw error. --- RserveVisualStudio/RPackage/RPackage.vcxproj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/RserveVisualStudio/RPackage/RPackage.vcxproj b/RserveVisualStudio/RPackage/RPackage.vcxproj index 65805ab..5d88a1c 100644 --- a/RserveVisualStudio/RPackage/RPackage.vcxproj +++ b/RserveVisualStudio/RPackage/RPackage.vcxproj @@ -90,7 +90,7 @@ copy $(SolutionDir)$(Platform)\$(Configuration)\*.exe $(SolutionDir)\..\Rserve\inst cd $(SolutionDir)\.. mkdir RServetempLibrary -R CMD INSTALL --no-multiarch --no-libs --no-test-load --build --library=RServetempLibrary Rserve +R CMD INSTALL --no-multiarch --no-libs --no-test-load --no-configure --build --library=RServetempLibrary Rserve rmdir /s /q RServetempLibrary $(SolutiontDir)\..\*.zip @@ -131,8 +131,8 @@ rmdir /s /q RServetempLibrary mkdir $(SolutionDir)\..\Rserve\inst copy $(SolutionDir)$(Platform)\$(Configuration)\*.exe $(SolutionDir)\..\Rserve\inst cd $(SolutionDir)\.. -mkdir RServetempLibrary -R CMD INSTALL --no-multiarch --no-libs --no-test-load --build --library=RServetempLibrary Rserve +mkdir RServetempLibrary +R CMD INSTALL --no-multiarch --no-libs --no-test-load --no-configure --build --library=RServetempLibrary Rserve rmdir /s /q RServetempLibrary $(SolutiontDir)\..\*.zip From ffb77d0b572f92951688d8cdb0f6079882d46695 Mon Sep 17 00:00:00 2001 From: CBA Date: Fri, 24 Mar 2017 17:30:26 +0100 Subject: [PATCH 3/8] Creation of child process failed because socket duplication was done in child but must be done in parent. The child threw socket error 10038. Doc of WSADuplicateSocket states it must be called in parent process. See also (here)[https://memset.wordpress.com/2010/10/13/win32-api-passing-socket-with-ipc-method/] on how to do it correctly and (here)[https://msdn.microsoft.com/en-us/library/windows/desktop/ms682499(v=vs.85).aspx] on how to use STDIN to talk to child process. --- Rserve/src/Rserv.c | 102 +++++++++++++++++++++++++++++---------------- 1 file changed, 67 insertions(+), 35 deletions(-) diff --git a/Rserve/src/Rserv.c b/Rserve/src/Rserv.c index b307d1a..7867871 100644 --- a/Rserve/src/Rserv.c +++ b/Rserve/src/Rserv.c @@ -382,7 +382,11 @@ int wfork(int socket, char* parentCmdLine, int idx) { return -1; } - char buf[128]; + char *cmdline = (char *)malloc(2048 * sizeof(char)); + if (cmdline == NULL) + { + return -1; + } size_t rc; BOOL bSuccess = FALSE; int m; @@ -392,13 +396,31 @@ int wfork(int socket, char* parentCmdLine, int idx) saAttr.bInheritHandle = TRUE; saAttr.lpSecurityDescriptor = NULL; + // Create pipes for sending data to the child process. + HANDLE hChildStd_IN_Rd; + HANDLE hChildStd_IN_Wr; + if (!CreatePipe(&hChildStd_IN_Rd, &hChildStd_IN_Wr, &saAttr, 0)) + return -1; + if (!SetHandleInformation(hChildStd_IN_Wr, HANDLE_FLAG_INHERIT, 0)) + return -1; + HANDLE hChildStd_OUT_Rd; + HANDLE hChildStd_OUT_Wr; + if (!CreatePipe(&hChildStd_OUT_Rd, &hChildStd_OUT_Wr, &saAttr, 0)) + return -1; + if (!SetHandleInformation(hChildStd_OUT_Wr, HANDLE_FLAG_INHERIT, 0)) + return -1; + // Set up members of the STARTUPINFO structure. // This structure specifies the STDIN and STDOUT handles for redirection. ZeroMemory (&siStartInfo, sizeof (STARTUPINFO)); siStartInfo.cb = sizeof (STARTUPINFO); + siStartInfo.hStdInput = hChildStd_IN_Rd; + siStartInfo.hStdOutput = hChildStd_OUT_Wr; + siStartInfo.hStdError = hChildStd_OUT_Wr; + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; //get the module name of the current process - if (!GetModuleFileNameA (GetModuleHandle (NULL), modname, 512)) + if (!GetModuleFileNameA (GetModuleHandle (NULL), modname, 2048)) { rc = GetLastError (); return -1; @@ -410,14 +432,13 @@ int wfork(int socket, char* parentCmdLine, int idx) winSocks[idx] = socket; //create the command line - sprintf_s (buf, 128, "%d --ppid %d", socket, (int)GetCurrentProcessId()); - strcat_s (modname, 2048, " --win32child "); - strcat_s (modname, 2048, buf); - strcat_s (modname, 2048, parentCmdLine); + sprintf_s (cmdline, 128, " --win32child --ppid %d", (int)GetCurrentProcessId()); + strcat_s (cmdline, 2048, parentCmdLine); + printf("start child process: %s %s\n", modname, cmdline); // Create the child process. - bSuccess = CreateProcessA ("rserve", - modname, // command line + bSuccess = CreateProcessA (modname, + cmdline, // command line NULL, // process security attributes NULL, // primary thread security attributes TRUE, // handles are inherited @@ -434,13 +455,32 @@ int wfork(int socket, char* parentCmdLine, int idx) ReleaseMutex(ghMutex); free (modname); + free (cmdline); // If an error occurs, exit the application. if (!bSuccess) { printf ("CreateProcess Failed.\n"); + printLastError(); return -1; } + else + { + // We want the child process to process the request coming in from socket. + // For this to work we need to create a duplicate that is attached to the child process. + // The duplication has to be done in the parent process. See doc of WSADuplicateSocket for details. + WSAPROTOCOL_INFO pi; + DWORD dwBytes; + if (WSADuplicateSocket((SOCKET)socket, winPI[idx].dwProcessId, &pi)) + { + int rc = WSAGetLastError(); + printf("rc_WSADuplicateSocket=%d\n", rc); + return -1; + } + // Write the protocol info of the duplicated socket to the STDIN of the child process. + if (!WriteFile(hChildStd_IN_Wr, &pi, sizeof(pi), &dwBytes, NULL)) + return -1; + } printf("create handles... Process = %d Thread = %d \n", (int)winPI[idx].hProcess, (int)winPI[idx].hThread); return (int)(winPI[idx]).hProcess; @@ -3562,42 +3602,34 @@ int main(int argc, char **argv) else MAX_CLIENTS = satoi(argv[++i]); } +#ifdef WIN32 //used only when launching Win32 child process via CreateProcess if (!strcmp(argv[i] + 2, "win32child")) { iWin32Child = 1; - if (i + 1 == argc) + // We expect the parent process to send us the duplicated socket for the incoming request by STDIN. + WSAPROTOCOL_INFO pi; + DWORD dwBytes; + HANDLE hStdin = GetStdHandle(STD_INPUT_HANDLE); + if (!ReadFile(hStdin, &pi, sizeof(pi), &dwBytes, NULL)) { - fprintf(stderr, "Missing socket specification for --win32child.\n"); + printf("Failed to get socket from parent process\n"); + return -1; + } + + SOCKET socket_duplicate = 0; + if ((socket_duplicate = WSASocket(pi.iAddressFamily, pi.iSocketType, pi.iProtocol, &pi, 0, 0)) != INVALID_SOCKET) + { + printf("WSASocket=%d\n", socket_duplicate); } else { -#ifdef WIN32 - socket = atoi(argv[++i]); - WSAPROTOCOL_INFO pi; - - if (WSADuplicateSocket((SOCKET)socket, GetCurrentProcessId(), &pi)) - { - int rc = WSAGetLastError(); - printf("rc_WSADuplicateSocket=%d\n", rc); - return -1; - } - - SOCKET socket_duplicate = 0; - if ((socket_duplicate = WSASocket(pi.iAddressFamily, pi.iSocketType, pi.iProtocol, &pi, 0, 0)) != INVALID_SOCKET) - { - printf("WSASocket=%d\n", socket_duplicate); - } - else - { - int rc = WSAGetLastError(); - printf("rc_WSASocket=%d\n", rc); - return -1; - } - socket = socket_duplicate; - -#endif + int rc = WSAGetLastError(); + printf("rc_WSASocket=%d\n", rc); + return -1; } + socket = socket_duplicate; } +#endif if (!strcmp(argv[i] + 2, "ppid")) { if (i + 1 == argc) fprintf(stderr, "Missing parent PID specification for --ppid.\n"); From 3d96024d743355c610403df0f8d82166799be68a Mon Sep 17 00:00:00 2001 From: CBA Date: Mon, 27 Mar 2017 13:09:45 +0200 Subject: [PATCH 4/8] Use double quotes around path of Rserve.exe when starting within R to ensure it works when path contains spaces. --- Rserve/R/conn.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Rserve/R/conn.R b/Rserve/R/conn.R index b7e179a..3e04722 100644 --- a/Rserve/R/conn.R +++ b/Rserve/R/conn.R @@ -5,7 +5,8 @@ Rserve <- function(debug=FALSE, port=6311, args=NULL) { if (!nchar(fn) || !file.exists(fn)) stop("Cannot find ", ffn) else { - if ( port != 6311 ) fn <- paste( fn, "--RS-port", port ) + fn <- paste("\"", fn, "\"", sep=''); + if ( port != 6311 ) fn <- paste( fn, "--RS-port", port ) if ( !is.null(args) ) fn <- paste(fn, paste(args, collapse=' ')) pad <- paste(R.home(),"\\bin;",sep='') From 793fe5728706d0abba047eb5a474e1574372a3e9 Mon Sep 17 00:00:00 2001 From: CBA Date: Mon, 27 Mar 2017 13:13:15 +0200 Subject: [PATCH 5/8] When starting Rserve from within R, always pass the port as parameter. Previously, it was not passed if port=6311. This led to use of the port 7004, the default port of Rserve.exe. (Why is the default port no longer 6311 anyway?) --- Rserve/R/conn.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rserve/R/conn.R b/Rserve/R/conn.R index 3e04722..42c24b7 100644 --- a/Rserve/R/conn.R +++ b/Rserve/R/conn.R @@ -6,7 +6,7 @@ Rserve <- function(debug=FALSE, port=6311, args=NULL) { stop("Cannot find ", ffn) else { fn <- paste("\"", fn, "\"", sep=''); - if ( port != 6311 ) fn <- paste( fn, "--RS-port", port ) + fn <- paste( fn, "--RS-port", port ) if ( !is.null(args) ) fn <- paste(fn, paste(args, collapse=' ')) pad <- paste(R.home(),"\\bin;",sep='') From b7ad7113f7d7bc712f21154dd12a6455c4c290ec Mon Sep 17 00:00:00 2001 From: CBA Date: Mon, 27 Mar 2017 13:38:06 +0200 Subject: [PATCH 6/8] Enhance include path for release build target. --- RserveVisualStudio/Rserve/Rserve.vcxproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RserveVisualStudio/Rserve/Rserve.vcxproj b/RserveVisualStudio/Rserve/Rserve.vcxproj index 56c72e8..1e71817 100644 --- a/RserveVisualStudio/Rserve/Rserve.vcxproj +++ b/RserveVisualStudio/Rserve/Rserve.vcxproj @@ -136,7 +136,7 @@ true true WIN32;NDEBUG;_CONSOLE;_LIB;Win32;_R_;NO_CONFIG_H;_SECURE;%(PreprocessorDefinitions) - $(ProjectDir);$(ProjectDir)..\..\Rserve\src;$(ProjectDir)..\..\Rserve\src\include;C:\Program Files\Microsoft\R Server\R_SERVER\include;C:\Program Files\Microsoft\R Server\R_SERVER\include\R_ext + $(ProjectDir);$(ProjectDir)..\..\Rserve\src;$(ProjectDir)..\..\Rserve\src\include;C:\Program Files\Microsoft\R Server\R_SERVER\include;C:\Program Files\Microsoft\R Server\R_SERVER\include\R_ext;c:\Program Files\Microsoft\MRO-3.3.2\include\ MultiThreaded From 7f1289aeb061c7ff1e9c9388dd646ec390766727 Mon Sep 17 00:00:00 2001 From: CBA Date: Mon, 27 Mar 2017 14:18:44 +0200 Subject: [PATCH 7/8] Ignore Visual Studio artifacts --- .gitignore | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.gitignore b/.gitignore index 2c29e12..ec80e1e 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,10 @@ *.esproj *.sublime-workspace *.sublime-project + +# Visual Studio artifacts +.vs/ +*.exe +x64/ +Debug/ +*.vcxproj.user From 99f7e602f88b0d45ed59ea1d7c480759dc239c23 Mon Sep 17 00:00:00 2001 From: CBA Date: Thu, 11 May 2017 09:19:47 +0200 Subject: [PATCH 8/8] Fixed setup of stdout pipe. --- Rserve/src/Rserv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rserve/src/Rserv.c b/Rserve/src/Rserv.c index 53971fe..96650ac 100644 --- a/Rserve/src/Rserv.c +++ b/Rserve/src/Rserv.c @@ -505,7 +505,7 @@ int wfork(int socket, char* parentCmdLine, int idx) HANDLE hChildStd_OUT_Wr; if (!CreatePipe(&hChildStd_OUT_Rd, &hChildStd_OUT_Wr, &saAttr, 0)) return -1; - if (!SetHandleInformation(hChildStd_OUT_Wr, HANDLE_FLAG_INHERIT, 0)) + if (!SetHandleInformation(hChildStd_OUT_Rd, HANDLE_FLAG_INHERIT, 0)) return -1; // Set up members of the STARTUPINFO structure.