From 74dad004d143a2d3fbe4171d20e57cc9942ad89d Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 14 Nov 2011 18:27:34 -0500 Subject: [PATCH] GUnixInput/OutputStream: fix blocking methods to always block Previously, if you created a GUnixInputStream or GUnixOutputStream from a non-blocking file descriptor, it might sometimes return G_IO_ERROR_WOULD_BLOCK from g_input_stream_read/g_output_stream_write, which is wrong. Fix that. (Use the GPollableInput/OutputStream methods if you want non-blocking I/O.) Also, add a test for this to gio/tests/unix-streams. Also, fix the GError messages to say "Error reading from file descriptor", etc instead of "Error reading from unix" (which was presumably from a bad search and replace job). https://bugzilla.gnome.org/show_bug.cgi?id=626866 --- gio/gunixinputstream.c | 48 ++++++++++++++++++++--------------- gio/gunixoutputstream.c | 54 +++++++++++++++++++++++----------------- gio/tests/unix-streams.c | 24 ++++++++++++++++-- 3 files changed, 81 insertions(+), 45 deletions(-) diff --git a/gio/gunixinputstream.c b/gio/gunixinputstream.c index c2137280c..3d5f93806 100644 --- a/gio/gunixinputstream.c +++ b/gio/gunixinputstream.c @@ -361,21 +361,27 @@ g_unix_input_stream_read (GInputStream *stream, GError **error) { GUnixInputStream *unix_stream; - gssize res; + gssize res = -1; GPollFD poll_fds[2]; + int nfds; int poll_ret; unix_stream = G_UNIX_INPUT_STREAM (stream); + poll_fds[0].fd = unix_stream->priv->fd; + poll_fds[0].events = G_IO_IN; if (unix_stream->priv->is_pipe_or_socket && g_cancellable_make_pollfd (cancellable, &poll_fds[1])) + nfds = 2; + else + nfds = 1; + + while (1) { - poll_fds[0].fd = unix_stream->priv->fd; - poll_fds[0].events = G_IO_IN; + poll_fds[0].revents = poll_fds[1].revents = 0; do - poll_ret = g_poll (poll_fds, 2, -1); + poll_ret = g_poll (poll_fds, nfds, -1); while (poll_ret == -1 && errno == EINTR); - g_cancellable_release_fd (cancellable); if (poll_ret == -1) { @@ -383,33 +389,35 @@ g_unix_input_stream_read (GInputStream *stream, g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error reading from unix: %s"), + _("Error reading from file descriptor: %s"), g_strerror (errsv)); - return -1; + break; } - } - while (1) - { if (g_cancellable_set_error_if_cancelled (cancellable, error)) - return -1; + break; + + if (!poll_fds[0].revents) + continue; + res = read (unix_stream->priv->fd, buffer, count); if (res == -1) { int errsv = errno; - if (errsv == EINTR) + if (errsv == EINTR || errsv == EAGAIN) continue; - + g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error reading from unix: %s"), + _("Error reading from file descriptor: %s"), g_strerror (errsv)); } - + break; } + g_cancellable_release_fd (cancellable); return res; } @@ -436,7 +444,7 @@ g_unix_input_stream_close (GInputStream *stream, g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error closing unix: %s"), + _("Error closing file descriptor: %s"), g_strerror (errsv)); } break; @@ -476,12 +484,12 @@ read_async_cb (int fd, { int errsv = errno; - if (errsv == EINTR) - continue; + if (errsv == EINTR || errsv == EAGAIN) + return TRUE; g_set_error (&error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error reading from unix: %s"), + _("Error reading from file descriptor: %s"), g_strerror (errsv)); } break; @@ -631,7 +639,7 @@ close_async_cb (CloseAsyncData *data) g_set_error (&error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error closing unix: %s"), + _("Error closing file descriptor: %s"), g_strerror (errsv)); } break; diff --git a/gio/gunixoutputstream.c b/gio/gunixoutputstream.c index a04e46ee9..01c32591e 100644 --- a/gio/gunixoutputstream.c +++ b/gio/gunixoutputstream.c @@ -346,56 +346,64 @@ g_unix_output_stream_write (GOutputStream *stream, GError **error) { GUnixOutputStream *unix_stream; - gssize res; + gssize res = -1; GPollFD poll_fds[2]; + int nfds; int poll_ret; unix_stream = G_UNIX_OUTPUT_STREAM (stream); + poll_fds[0].fd = unix_stream->priv->fd; + poll_fds[0].events = G_IO_OUT; + if (unix_stream->priv->is_pipe_or_socket && g_cancellable_make_pollfd (cancellable, &poll_fds[1])) + nfds = 2; + else + nfds = 1; + + while (1) { - poll_fds[0].fd = unix_stream->priv->fd; - poll_fds[0].events = G_IO_OUT; + poll_fds[0].revents = poll_fds[1].revents = 0; do - poll_ret = g_poll (poll_fds, 2, -1); + poll_ret = g_poll (poll_fds, nfds, -1); while (poll_ret == -1 && errno == EINTR); - g_cancellable_release_fd (cancellable); - + if (poll_ret == -1) { int errsv = errno; g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error writing to unix: %s"), + _("Error writing to file descriptor: %s"), g_strerror (errsv)); - return -1; + break; } - } - - while (1) - { + if (g_cancellable_set_error_if_cancelled (cancellable, error)) - return -1; + break; + + if (!poll_fds[0].revents) + continue; res = write (unix_stream->priv->fd, buffer, count); if (res == -1) { int errsv = errno; - if (errsv == EINTR) + if (errsv == EINTR || errsv == EAGAIN) continue; - + g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error writing to unix: %s"), + _("Error writing to file descriptor: %s"), g_strerror (errsv)); } - + break; } - + + g_cancellable_release_fd (cancellable); return res; } @@ -422,7 +430,7 @@ g_unix_output_stream_close (GOutputStream *stream, g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error closing unix: %s"), + _("Error closing file descriptor: %s"), g_strerror (errsv)); } break; @@ -462,12 +470,12 @@ write_async_cb (int fd, { int errsv = errno; - if (errsv == EINTR) - continue; + if (errsv == EINTR || errsv == EAGAIN) + return TRUE; g_set_error (&error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error writing to unix: %s"), + _("Error writing to file descriptor: %s"), g_strerror (errsv)); } break; @@ -586,7 +594,7 @@ close_async_cb (CloseAsyncData *data) g_set_error (&error, G_IO_ERROR, g_io_error_from_errno (errsv), - _("Error closing unix: %s"), + _("Error closing file descriptor: %s"), g_strerror (errsv)); } break; diff --git a/gio/tests/unix-streams.c b/gio/tests/unix-streams.c index aaf8aa892..dc8cf3545 100644 --- a/gio/tests/unix-streams.c +++ b/gio/tests/unix-streams.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -194,7 +195,7 @@ timeout (gpointer cancellable) } static void -test_pipe_io (void) +test_pipe_io (gconstpointer nonblocking) { GThread *writer, *reader; GInputStream *in; @@ -212,6 +213,20 @@ test_pipe_io (void) g_assert (pipe (writer_pipe) == 0 && pipe (reader_pipe) == 0); + if (nonblocking) + { + GError *error = NULL; + + g_unix_set_fd_nonblocking (writer_pipe[0], TRUE, &error); + g_assert_no_error (error); + g_unix_set_fd_nonblocking (writer_pipe[1], TRUE, &error); + g_assert_no_error (error); + g_unix_set_fd_nonblocking (reader_pipe[0], TRUE, &error); + g_assert_no_error (error); + g_unix_set_fd_nonblocking (reader_pipe[1], TRUE, &error); + g_assert_no_error (error); + } + writer_cancel = g_cancellable_new (); reader_cancel = g_cancellable_new (); main_cancel = g_cancellable_new (); @@ -249,7 +264,12 @@ main (int argc, g_type_init (); g_test_init (&argc, &argv, NULL); - g_test_add_func ("/unix-streams/pipe-io-test", test_pipe_io); + g_test_add_data_func ("/unix-streams/pipe-io-test", + GINT_TO_POINTER (FALSE), + test_pipe_io); + g_test_add_data_func ("/unix-streams/nonblocking-io-test", + GINT_TO_POINTER (TRUE), + test_pipe_io); return g_test_run(); }