Skip to content

Commit 9a4e2e8

Browse files
hanumantmkbjori
hanumantmk
authored and
bjori
committed
CDRIVER-756: Unchecked errors on failed network writes
1 parent f2ee4c3 commit 9a4e2e8

File tree

7 files changed

+224
-82
lines changed

7 files changed

+224
-82
lines changed

‎src/mongoc/mongoc-cluster.c

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -954,9 +954,9 @@ _mongoc_cluster_run_command (mongoc_cluster_t *cluster,
954954
_mongoc_rpc_swab_to_le(&rpc);
955955

956956
DUMP_IOVEC (((mongoc_iovec_t *)ar.data), ((mongoc_iovec_t *)ar.data), ar.len);
957-
if (!mongoc_stream_writev(node->stream, ar.data, ar.len,
958-
cluster->sockettimeoutms)) {
959-
GOTO(failure);
957+
if (!_mongoc_stream_writev_full (node->stream, ar.data, ar.len,
958+
cluster->sockettimeoutms, error)) {
959+
GOTO (failure);
960960
}
961961

962962
if (!_mongoc_buffer_append_from_stream(&buffer, node->stream, 4,
@@ -2896,17 +2896,8 @@ _mongoc_cluster_sendv (mongoc_cluster_t *cluster,
28962896

28972897
BSON_ASSERT (cluster->iov.len);
28982898

2899-
if (!mongoc_stream_writev (node->stream, iov, iovcnt,
2900-
cluster->sockettimeoutms)) {
2901-
char buf[128];
2902-
char * errstr;
2903-
errstr = bson_strerror_r(errno, buf, sizeof buf);
2904-
2905-
bson_set_error (error,
2906-
MONGOC_ERROR_STREAM,
2907-
MONGOC_ERROR_STREAM_SOCKET,
2908-
"Failure during socket delivery: %s",
2909-
errstr);
2899+
if (!_mongoc_stream_writev_full (node->stream, iov, iovcnt,
2900+
cluster->sockettimeoutms, error)) {
29102901
_mongoc_cluster_disconnect_node (cluster, node);
29112902
RETURN (0);
29122903
}
@@ -3037,17 +3028,8 @@ _mongoc_cluster_try_sendv (mongoc_cluster_t *cluster,
30373028

30383029
DUMP_IOVEC (iov, iov, iovcnt);
30393030

3040-
if (!mongoc_stream_writev (node->stream, iov, iovcnt,
3041-
cluster->sockettimeoutms)) {
3042-
char buf[128];
3043-
char * errstr;
3044-
errstr = bson_strerror_r(errno, buf, sizeof buf);
3045-
3046-
bson_set_error (error,
3047-
MONGOC_ERROR_STREAM,
3048-
MONGOC_ERROR_STREAM_SOCKET,
3049-
"Failure during socket delivery: %s",
3050-
errstr);
3031+
if (!_mongoc_stream_writev_full (node->stream, iov, iovcnt,
3032+
cluster->sockettimeoutms, error)) {
30513033
_mongoc_cluster_disconnect_node (cluster, node);
30523034
RETURN (0);
30533035
}

‎src/mongoc/mongoc-iovec.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,12 @@
2121

2222
#include <bson.h>
2323

24-
#ifndef _WIN32
24+
#ifdef _WIN32
25+
# include <stddef.h>
26+
#else
2527
# include <sys/uio.h>
2628
#endif
2729

28-
2930
BSON_BEGIN_DECLS
3031

3132

@@ -35,6 +36,11 @@ typedef struct
3536
u_long iov_len;
3637
char *iov_base;
3738
} mongoc_iovec_t;
39+
40+
BSON_STATIC_ASSERT(sizeof(mongoc_iovec_t) == sizeof(WSABUF));
41+
BSON_STATIC_ASSERT(offsetof(mongoc_iovec_t, iov_base) == offsetof(WSABUF, buf));
42+
BSON_STATIC_ASSERT(offsetof(mongoc_iovec_t, iov_len) == offsetof(WSABUF, len));
43+
3844
#else
3945
typedef struct iovec mongoc_iovec_t;
4046
#endif

‎src/mongoc/mongoc-socket.c

Lines changed: 74 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -119,22 +119,12 @@ _mongoc_socket_wait (int sd, /* IN */
119119
#endif
120120
int ret;
121121
int timeout;
122+
int64_t now;
122123

123124
ENTRY;
124125

125126
bson_return_val_if_fail (events, false);
126127

127-
if (expire_at < 0) {
128-
timeout = -1;
129-
} else if (expire_at == 0) {
130-
timeout = 0;
131-
} else {
132-
timeout = (int)((expire_at - bson_get_monotonic_time ()) / 1000L);
133-
if (timeout < 0) {
134-
timeout = 0;
135-
}
136-
}
137-
138128
pfd.fd = sd;
139129
#ifdef _WIN32
140130
pfd.events = events;
@@ -143,25 +133,57 @@ _mongoc_socket_wait (int sd, /* IN */
143133
#endif
144134
pfd.revents = 0;
145135

136+
now = bson_get_monotonic_time();
137+
138+
for (;;) {
139+
if (expire_at < 0) {
140+
timeout = -1;
141+
} else if (expire_at == 0) {
142+
timeout = 0;
143+
} else {
144+
timeout = (int)((expire_at - now) / 1000L);
145+
if (timeout < 0) {
146+
timeout = 0;
147+
}
148+
}
149+
146150
#ifdef _WIN32
147-
ret = WSAPoll (&pfd, 1, timeout);
148-
if (ret == SOCKET_ERROR) {
149-
MONGOC_WARNING ("WSAGetLastError(): %d", WSAGetLastError ());
150-
ret = false;
151-
}
151+
ret = WSAPoll (&pfd, 1, timeout);
152+
if (ret == SOCKET_ERROR) {
153+
errno = WSAGetLastError();
154+
ret = -1;
155+
}
152156
#else
153-
ret = poll (&pfd, 1, timeout);
157+
ret = poll (&pfd, 1, timeout);
154158
#endif
155159

156-
if (ret > 0) {
160+
if (ret > 0) {
161+
/* Something happened, so return that */
157162
#ifdef _WIN32
158-
RETURN (0 != (pfd.revents & (events | POLLHUP | POLLERR)));
163+
RETURN (0 != (pfd.revents & (events | POLLHUP | POLLERR)));
159164
#else
160-
RETURN (0 != (pfd.revents & events));
165+
RETURN (0 != (pfd.revents & events));
161166
#endif
162-
}
167+
} else if (ret < 0) {
168+
/* poll itself failed */
163169

164-
RETURN (false);
170+
if (MONGOC_ERRNO_IS_AGAIN(errno)) {
171+
now = bson_get_monotonic_time();
172+
173+
if (expire_at < now) {
174+
RETURN (false);
175+
} else {
176+
continue;
177+
}
178+
} else {
179+
/* poll failed for some non-transient reason */
180+
RETURN (false);
181+
}
182+
} else {
183+
/* poll timed out */
184+
RETURN (false);
185+
}
186+
}
165187
}
166188

167189

@@ -417,16 +439,18 @@ mongoc_socket_close (mongoc_socket_t *sock) /* IN */
417439
if (sock->sd != INVALID_SOCKET) {
418440
shutdown (sock->sd, SD_BOTH);
419441
ret = closesocket (sock->sd);
442+
} else {
443+
RETURN(0);
420444
}
421445
#else
422446
if (sock->sd != -1) {
423447
shutdown (sock->sd, SHUT_RDWR);
424448
ret = close (sock->sd);
449+
} else {
450+
RETURN(0);
425451
}
426452
#endif
427453

428-
_mongoc_socket_capture_errno (sock);
429-
430454
if (ret == 0) {
431455
#ifdef _WIN32
432456
sock->sd = INVALID_SOCKET;
@@ -436,6 +460,8 @@ mongoc_socket_close (mongoc_socket_t *sock) /* IN */
436460
RETURN (0);
437461
}
438462

463+
_mongoc_socket_capture_errno (sock);
464+
439465
RETURN (-1);
440466
}
441467

@@ -477,13 +503,13 @@ mongoc_socket_connect (mongoc_socket_t *sock, /* IN */
477503

478504
ret = connect (sock->sd, addr, addrlen);
479505

480-
_mongoc_socket_capture_errno (sock);
481-
482506
#ifdef _WIN32
483507
if (ret == SOCKET_ERROR) {
484508
#else
485509
if (ret == -1) {
486510
#endif
511+
_mongoc_socket_capture_errno (sock);
512+
487513
failed = true;
488514
try_again = _mongoc_socket_errno_is_again (sock);
489515
}
@@ -692,7 +718,9 @@ mongoc_socket_recv (mongoc_socket_t *sock, /* IN */
692718
ret = recv (sock->sd, buf, buflen, flags);
693719
failed = (ret == -1);
694720
#endif
695-
_mongoc_socket_capture_errno (sock);
721+
if (failed) {
722+
_mongoc_socket_capture_errno (sock);
723+
}
696724
try_again = (failed && _mongoc_socket_errno_is_again (sock));
697725

698726
if (failed && try_again) {
@@ -707,7 +735,7 @@ mongoc_socket_recv (mongoc_socket_t *sock, /* IN */
707735

708736
DUMP_BYTES (recvbuf, (uint8_t *)buf, ret);
709737

710-
mongoc_counter_streams_ingress_add (ret > 0 ? ret : 0);
738+
mongoc_counter_streams_ingress_add (ret);
711739

712740
RETURN (ret);
713741
}
@@ -795,9 +823,8 @@ mongoc_socket_send (mongoc_socket_t *sock, /* IN */
795823
* _mongoc_socket_try_sendv_slow --
796824
*
797825
* A slow variant of _mongoc_socket_try_sendv() that sends each
798-
* iovec entry one by one. This can happen if we hit EMSGSIZE on
799-
* with sendmsg() on various POSIX systems (such as Solaris), or
800-
* on WinXP.
826+
* iovec entry one by one. This can happen if we hit EMSGSIZE
827+
* with sendmsg() on various POSIX systems.
801828
*
802829
* Returns:
803830
* the number of bytes sent or -1 and errno is set.
@@ -825,12 +852,13 @@ _mongoc_socket_try_sendv_slow (mongoc_socket_t *sock, /* IN */
825852

826853
for (i = 0; i < iovcnt; i++) {
827854
wrote = send (sock->sd, iov [i].iov_base, iov [i].iov_len, 0);
828-
_mongoc_socket_capture_errno (sock);
829855
#ifdef _WIN32
830856
if (wrote == SOCKET_ERROR) {
831857
#else
832858
if (wrote == -1) {
833859
#endif
860+
_mongoc_socket_capture_errno (sock);
861+
834862
if (!_mongoc_socket_errno_is_again (sock)) {
835863
RETURN (-1);
836864
}
@@ -913,7 +941,7 @@ _mongoc_socket_try_sendv (mongoc_socket_t *sock, /* IN */
913941
#else
914942
if ((ret == -1) && (errno == EMSGSIZE)) {
915943
#endif
916-
_mongoc_socket_try_sendv_slow (sock, iov, iovcnt);
944+
RETURN(_mongoc_socket_try_sendv_slow (sock, iov, iovcnt));
917945
}
918946

919947
_mongoc_socket_capture_errno (sock);
@@ -947,20 +975,24 @@ _mongoc_socket_try_sendv (mongoc_socket_t *sock, /* IN */
947975

948976
ssize_t
949977
mongoc_socket_sendv (mongoc_socket_t *sock, /* IN */
950-
mongoc_iovec_t *iov, /* IN */
978+
mongoc_iovec_t *in_iov, /* IN */
951979
size_t iovcnt, /* IN */
952980
int64_t expire_at) /* IN */
953981
{
954982
ssize_t ret = 0;
955983
ssize_t sent;
956984
size_t cur = 0;
985+
mongoc_iovec_t *iov;
957986

958987
ENTRY;
959988

960989
bson_return_val_if_fail (sock, -1);
961-
bson_return_val_if_fail (iov, -1);
990+
bson_return_val_if_fail (in_iov, -1);
962991
bson_return_val_if_fail (iovcnt, -1);
963992

993+
iov = bson_malloc(sizeof(*iov) * iovcnt);
994+
memcpy(iov, in_iov, sizeof(*iov) * iovcnt);
995+
964996
for (;;) {
965997
sent = _mongoc_socket_try_sendv (sock, &iov [cur], iovcnt - cur);
966998

@@ -971,7 +1003,8 @@ mongoc_socket_sendv (mongoc_socket_t *sock, /* IN */
9711003
*/
9721004
if (sent == -1) {
9731005
if (!_mongoc_socket_errno_is_again (sock)) {
974-
RETURN (ret ? ret : -1);
1006+
ret = -1;
1007+
goto CLEANUP;
9751008
}
9761009
}
9771010

@@ -1007,29 +1040,20 @@ mongoc_socket_sendv (mongoc_socket_t *sock, /* IN */
10071040
BSON_ASSERT (iovcnt - cur);
10081041
BSON_ASSERT (iov [cur].iov_len);
10091042
} else if (OPERATION_EXPIRED (expire_at)) {
1010-
#ifdef _WIN32
1011-
errno = WSAETIMEDOUT;
1012-
#else
1013-
errno = ETIMEDOUT;
1014-
#endif
1015-
RETURN (ret ? ret : -1);
1043+
goto CLEANUP;
10161044
}
10171045

10181046
/*
10191047
* Block on poll() until our desired condition is met.
10201048
*/
10211049
if (!_mongoc_socket_wait (sock->sd, POLLOUT, expire_at)) {
1022-
if (ret == 0){
1023-
#ifdef _WIN32
1024-
errno = WSAETIMEDOUT;
1025-
#else
1026-
errno = ETIMEDOUT;
1027-
#endif
1028-
}
1029-
RETURN (ret ? ret : -1);
1050+
goto CLEANUP;
10301051
}
10311052
}
10321053

1054+
CLEANUP:
1055+
bson_free(iov);
1056+
10331057
RETURN (ret);
10341058
}
10351059

‎src/mongoc/mongoc-stream-private.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ BSON_BEGIN_DECLS
3434
#define MONGOC_STREAM_GRIDFS 4
3535
#define MONGOC_STREAM_TLS 5
3636

37+
bool
38+
_mongoc_stream_writev_full (mongoc_stream_t *stream,
39+
mongoc_iovec_t *iov,
40+
size_t iovcnt,
41+
int32_t timeout_msec,
42+
bson_error_t *error);
43+
3744

3845
BSON_END_DECLS
3946

‎src/mongoc/mongoc-stream-tls.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,6 @@ _mongoc_stream_tls_write (mongoc_stream_tls_t *tls,
480480
if ((expire - now) < 0) {
481481
if (ret < buf_len) {
482482
mongoc_counter_streams_timeout_inc();
483-
#ifdef _WIN32
484-
errno = WSAETIMEDOUT;
485-
#else
486-
errno = ETIMEDOUT;
487-
#endif
488483
}
489484

490485
tls->timeout_msec = 0;

0 commit comments

Comments
 (0)