>From 56a69318b55f0a231cf1531d2b7035c994df6579 Mon Sep 17 00:00:00 2001
From: Neil Jerram
Date: Sat, 27 Mar 2010 21:41:54 +0000
Subject: [PATCH] Fix windows sockets vs. file descriptors bugs
Patch is from Scott McPeak, modified by me to be as conservative as
possible with respect to existing 1.8.x non-MinGW operation.
Scott writes:
"Guile-1.8.7 appears to have socket support and Windows support, but
they don't seem to work together, despite the existence of things like
win32-socket.c. (I'm building Guile for Windows using the mingw cross
compiler on linux. Maybe it's different with Cygwin?)
Specifically, Guile assumes the POSIX rule that a socket is just a
file descriptor, but on Windows that does not work; socket functions
only accept sockets, and file functions only accept file descriptors.
Several Guile functions are affected; I don't think this is an
exhaustive list, but it's what I ran into while trying to get a simple
client and server working (see testcase below):
* fport_fill_input: Passes a socket to 'read'.
* write_all: Passes a socket to 'write'.
* fport_close: Passes a socket to 'close'. The EBADF error message is
then silently discarded (...), but the bug still manifests, e.g., as a
server that never closes its connections.
* scm_std_select: Passes a pipe file descriptor to 'select'.
I'm not sure what the best solution is in the Guile framework. I see
that ports have some sort of dynamic dispatch table, so perhaps
sockets should be made a distinct kind of port from a file port using
that mechanism. However, for expedience, I basically hacked the file
port functions by recognizing sockets as ports with a SCM_FILENAME
that is sym_socket and handling them differently. However, that is
certainly not perfect since 'set-port-filename!' can be used to change
the file name after creation.
Since there should be no harm in calling the socket-specific functions
for socket file descriptors on any operating system, and differences
in behavior among platforms make good hiding places for bugs, my patch
makes most of the hacks regardless of the platform. I've tested it on
linux/x86 and win32/x86.
For 'select', I don't understand the purpose of the sleep pipe, and,
whatever it does, it would likely be a lot of work to code it in a
Windows-compatible way, so I just removed it on Windows."
Regarding the sleep pipe, as I've written in a comment in threads.c...
The consequence of not implementing the sleep pipe (on MinGW) is that
threads cannot receive signals, or more generally any asyncs, while
they are blocked waiting for I/O (select etc.), or for a mutex or
condition variable. But that is better than those
operations (i.e. I/O, mutexes and condition variables) not working at
all!
* libguile/fports.c: Include libguile/socket.h.
(fport_fill_input): On MinGW, use scm_is_socket_port and
scm_socket_read_via_recv.
(write_all): On MinGW, use scm_is_socket_port and
scm_socket_write_via_send.
(fport_close): On MinGW, use scm_is_socket_port and
scm_socket_close.
* libguile/socket.c (scm_is_socket_port, scm_socket_read_via_recv,
scm_socket_write_via_send, scm_socket_close): New functions for
MinGW.
* libguile/socket.h: Corresponding declarations.
* libguile/threads.c (scm_std_select): On MinGW, suppress all the bits
to do with the sleep pipe.
---
libguile/fports.c | 29 +++++++++++++++++++++++++----
libguile/socket.c | 31 +++++++++++++++++++++++++++++++
libguile/socket.h | 9 +++++++++
libguile/threads.c | 29 ++++++++++++++++++++++++++++-
4 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/libguile/fports.c b/libguile/fports.c
index 007ee3f..c047a4e 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -26,6 +26,7 @@
#include
#include
#include "libguile/_scm.h"
+#include "libguile/socket.h"
#include "libguile/strings.h"
#include "libguile/validate.h"
#include "libguile/gc.h"
@@ -596,8 +597,14 @@ fport_fill_input (SCM port)
#ifndef __MINGW32__
fport_wait_for_input (port);
-#endif /* !__MINGW32__ */
- SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
+#else /* __MINGW32__ */
+ if (scm_is_socket_port (port))
+ SCM_SYSCALL (count =
+ scm_socket_read_via_recv (fp->fdes, pt->read_buf, pt->read_buf_size));
+ else
+#endif /* __MINGW32__ */
+ SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
+
if (count == -1)
scm_syserror ("fport_fill_input");
if (count == 0)
@@ -717,12 +724,20 @@ scm_i_fport_truncate (SCM port, SCM length)
static void write_all (SCM port, const void *data, size_t remaining)
{
int fdes = SCM_FSTREAM (port)->fdes;
+#ifdef __MINGW32__
+ int is_socket = scm_is_socket_port (port);
+#endif
while (remaining > 0)
{
size_t done;
- SCM_SYSCALL (done = write (fdes, data, remaining));
+#ifdef __MINGW32__
+ if (is_socket)
+ SCM_SYSCALL (done = scm_socket_write_via_send (fdes, data, remaining));
+ else
+#endif
+ SCM_SYSCALL (done = write (fdes, data, remaining));
if (done == -1)
SCM_SYSERROR;
@@ -880,7 +895,13 @@ fport_close (SCM port)
int rv;
fport_flush (port);
- SCM_SYSCALL (rv = close (fp->fdes));
+#ifdef __MINGW32__
+ if (scm_is_socket_port (port))
+ SCM_SYSCALL (rv = scm_socket_close (fp->fdes));
+ else
+#endif
+ SCM_SYSCALL (rv = close (fp->fdes));
+
if (rv == -1 && errno != EBADF)
{
if (scm_gc_running_p)
diff --git a/libguile/socket.c b/libguile/socket.c
index cb954f4..b41f839 100644
--- a/libguile/socket.c
+++ b/libguile/socket.c
@@ -1642,7 +1642,38 @@ SCM_DEFINE (scm_sendto, "sendto", 3, 1, 1,
#undef FUNC_NAME
+#ifdef __MINGW32__
+/* The functions in this section support using sockets on Windows,
+ * where the file descriptor functions cannot be used on sockets. */
+
+/* Return true if 'port' is a socket port. */
+int scm_is_socket_port(SCM port)
+{
+ return SCM_FILENAME (port) == sym_socket;
+}
+
+/* Do what POSIX 'read' system call would do, except for a file
+ * descriptor that is a socket. */
+int scm_socket_read_via_recv(int fd, void *buf, size_t len)
+{
+ return recv (fd, buf, len, 0 /*flags*/);
+}
+
+/* Like 'write' but for a socket. */
+int scm_socket_write_via_send(int fd, void const *buf, size_t len)
+{
+ return send (fd, buf, len, 0 /*flags*/);
+}
+
+/* Like 'close' but for a socket. */
+int scm_socket_close(int fd)
+{
+ return closesocket(fd);
+}
+#endif
+
+
void
scm_init_socket ()
{
diff --git a/libguile/socket.h b/libguile/socket.h
index 146d283..c8213eb 100644
--- a/libguile/socket.h
+++ b/libguile/socket.h
@@ -64,6 +64,15 @@ SCM_API struct sockaddr *scm_c_make_socket_address (SCM family, SCM address,
size_t *address_size);
SCM_API SCM scm_make_socket_address (SCM family, SCM address, SCM args);
+#ifdef __MINGW32__
+/* Socket functions on file descriptors, exported from socket.c so
+ * that fports.c does not have to know about system socket headers. */
+SCM_API int scm_is_socket_port(SCM port);
+SCM_API int scm_socket_read_via_recv(int fd, void *buf, size_t len);
+SCM_API int scm_socket_write_via_send(int fd, void const *buf, size_t len);
+SCM_API int scm_socket_close(int fd);
+#endif
+
#endif /* SCM_SOCKET_H */
/*
diff --git a/libguile/threads.c b/libguile/threads.c
index f2bb556..06aeb3c 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -1419,6 +1419,15 @@ scm_threads_mark_stacks (void)
/*** Select */
+/* On Windows, we cannot use 'select' with non-socket file
+ * descriptors, so we cannot use the sleep pipe. The consequence of
+ * this is that threads cannot receive signals, or more generally any
+ * asyncs, while they are blocked waiting for I/O (select etc.), or
+ * for a mutex or condition variable. But that is better than those
+ * operations (i.e. I/O, mutexes and condition variables) not working
+ * at all!
+ */
+
int
scm_std_select (int nfds,
SELECT_TYPE *readfds,
@@ -1437,19 +1446,35 @@ scm_std_select (int nfds,
readfds = &my_readfds;
}
+#ifndef __MINGW32__
while (scm_i_setup_sleep (t, SCM_BOOL_F, NULL, t->sleep_pipe[1]))
SCM_TICK;
wakeup_fd = t->sleep_pipe[0];
+#endif
+
ticket = scm_leave_guile ();
+
+#ifndef __MINGW32__
FD_SET (wakeup_fd, readfds);
if (wakeup_fd >= nfds)
nfds = wakeup_fd+1;
+#endif
+
res = select (nfds, readfds, writefds, exceptfds, timeout);
- t->sleep_fd = -1;
eno = errno;
+
+#ifndef __MINGW32__
+ /* XXX: Is it important to set 'sleep_fd' before re-entering Guile
+ * mode? If not, then this assignment should move down into the
+ * next 'if' block for simplicity. But in that case, there is no
+ * point since 'scm_i_reset_sleep' also sets the field to -1. */
+ t->sleep_fd = -1;
+#endif
+
scm_enter_guile (ticket);
+#ifndef __MINGW32__
scm_i_reset_sleep (t);
if (res > 0 && FD_ISSET (wakeup_fd, readfds))
@@ -1467,6 +1492,8 @@ scm_std_select (int nfds,
res = -1;
}
}
+#endif
+
errno = eno;
return res;
}
--
1.5.6.5