Skip to content

Commit 7deac02

Browse files
committed
Unix(macOS) SocketPlugin:
Fix sendDone/primtiveSocketSendDone/sqSocketSendDone on macOS. The select in socketWritable does not answer true for writable sockets, presumably because the select in aioPoll already has done. So have dataHandler (which is called from aioPoll when the socket *is* writable) set a flag in privateSocketStruct notifiedOfWritability and use this to avoid the wrongly unsuccessful call of select in sqSocketSendDone. Clear notifiedOfWritability before any and all send/ write system calls. Have sqWin32NewNet.c process the select result in its socketWritable in exactly the same way as sqUnixSocket.c's. I WELCOME ANY REVIEW OF THIS COMMIT!! Levente, Tobias, you might take a close look at this one.
1 parent 3f129f2 commit 7deac02

File tree

2 files changed

+46
-20
lines changed

2 files changed

+46
-20
lines changed

platforms/unix/plugins/SocketPlugin/sqUnixSocket.c

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -165,18 +165,19 @@ union sockaddr_any
165165

166166
typedef struct privateSocketStruct
167167
{
168-
int s; /* Unix socket */
169-
int connSema; /* connection io notification semaphore */
170-
int readSema; /* read io notification semaphore */
171-
int writeSema; /* write io notification semaphore */
172-
int sockState; /* connection + data state */
173-
int sockError; /* errno after socket error */
168+
int s; /* Unix socket */
169+
int connSema; /* connection io notification semaphore */
170+
int readSema; /* read io notification semaphore */
171+
int writeSema; /* write io notification semaphore */
172+
int sockState; /* connection + data state */
173+
int sockError; /* errno after socket error */
174174
union sockaddr_any peer; /* default send/recv address for UDP */
175175
socklen_t peerSize; /* dynamic sizeof(peer) */
176-
union sockaddr_any sender; /* sender address for last UDP receive */
176+
union sockaddr_any sender;/* sender address for last UDP receive */
177177
socklen_t senderSize; /* dynamic sizeof(sender) */
178-
int multiListen; /* whether to listen for multiple connections */
179-
int acceptedSock; /* a connection that has been accepted */
178+
int multiListen; /* whether to listen for multiple connections */
179+
int acceptedSock; /* a connection that has been accepted */
180+
int notifiedOfWritability; /* flag compensates for select denying writable */
180181
} privateSocketStruct;
181182

182183
#define CONN_NOTIFY (1<<0)
@@ -343,12 +344,20 @@ socketReadable(int s)
343344
static int
344345
socketWritable(int s)
345346
{
346-
struct timeval tv= { 0, 0 };
347+
struct timeval tv = { 0, 0 }; // i.e. poll
347348
fd_set fds;
348349

349350
FD_ZERO(&fds);
350351
FD_SET(s, &fds);
351-
return select(s+1, 0, &fds, 0, &tv) > 0;
352+
#ifdef AIO_DEBUG
353+
{ int r = select(1, 0, &fds, 0, &tv);
354+
if (r < 0)
355+
perror("socketWritable: select(1,0,&fd,0,&poll)");
356+
return r > 0;
357+
}
358+
#else
359+
return select(1, 0, &fds, 0, &tv) > 0;
360+
#endif
352361
}
353362

354363
/* answer the error condition on the given socket */
@@ -488,7 +497,14 @@ dataHandler(int fd, void *data, int flags)
488497
}
489498
notify(pss, READ_NOTIFY);
490499
}
491-
if (flags & AIO_W) notify(pss, WRITE_NOTIFY);
500+
if (flags & AIO_W) {
501+
notify(pss, WRITE_NOTIFY);
502+
/* in socketWritable select may answer zero for writable sockets, resulting
503+
* in sqSocketSendDone answering false for new sockets. So we subvert select
504+
* by setting the notifiedOfWritability flag, tested in sqSocketSendDone.
505+
*/
506+
pss->notifiedOfWritability = true;
507+
}
492508
if (flags & AIO_X)
493509
{
494510
/* assume out-of-band data has arrived */
@@ -1120,16 +1136,23 @@ sqSocketReceiveDataAvailable(SocketPtr s)
11201136
}
11211137

11221138

1123-
/* answer whether the socket has space to receive more data */
1139+
/* answer whether the socket has space to send more data */
11241140

11251141
sqInt
11261142
sqSocketSendDone(SocketPtr s)
11271143
{
1128-
if (!socketValid(s))
1129-
return false;
1130-
if (SOCKETSTATE(s) == Connected)
1131-
{
1132-
if (socketWritable(SOCKET(s))) return true;
1144+
if (socketValid(s)
1145+
&& SOCKETSTATE(s) == Connected)
1146+
{
1147+
/* in socketWritable select may answer zero for writable sockets, resulting
1148+
* in sqSocketSendDone answering false for new sockets. So we subvert select
1149+
* by testing the notifiedOfWritability flag, set in dataHandler.
1150+
*/
1151+
if (PSP(s)->notifiedOfWritability)
1152+
return true;
1153+
if (socketWritable(SOCKET(s)))
1154+
return true;
1155+
FPRINTF((stderr, "sqSocketSendDone(%d) !socketWritable\n", SOCKET(s)));
11331156
aioHandle(SOCKET(s), dataHandler, AIO_WX);
11341157
}
11351158
return false;
@@ -1202,6 +1225,7 @@ sqSocketSendDataBufCount(SocketPtr s, char *buf, sqInt bufSize)
12021225
if (!socketValid(s))
12031226
return -1;
12041227

1228+
PSP(s)->notifiedOfWritability = false;
12051229
if (TCPSocketType != s->socketType)
12061230
{
12071231
/* --- UDP/RAW --- */
@@ -1291,6 +1315,7 @@ sqSockettoHostportSendDataBufCount(SocketPtr s, sqInt address, sqInt port, char
12911315
saddr.sin_family= AF_INET;
12921316
saddr.sin_port= htons((short)port);
12931317
saddr.sin_addr.s_addr= htonl(address);
1318+
PSP(s)->notifiedOfWritability = false;
12941319
{
12951320
int nsent= sendto(SOCKET(s), buf, bufSize, 0, (struct sockaddr *)&saddr, sizeof(saddr));
12961321
if (nsent >= 0)
@@ -2348,6 +2373,7 @@ sqSocketSendUDPToSizeDataBufCount(SocketPtr s, char *addr, sqInt addrSize, char
23482373
if (socketValid(s) && addressValid(addr, addrSize) && (TCPSocketType != s->socketType)) /* --- UDP/RAW --- */
23492374
{
23502375
int nsent= sendto(SOCKET(s), buf, bufSize, 0, socketAddress(addr), addrSize - AddressHeaderSize);
2376+
PSP(s)->notifiedOfWritability = false;
23512377
if (nsent >= 0)
23522378
return nsent;
23532379

platforms/win32/plugins/SocketPlugin/sqWin32NewNet.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,12 @@ static int socketReadable(SOCKET s)
186186

187187
static int socketWritable(SOCKET s)
188188
{
189-
struct timeval tv= { 0, 0 };
189+
struct timeval tv = { 0, 0 }; // i.e. poll
190190
fd_set fds;
191191

192192
FD_ZERO(&fds);
193193
FD_SET(s, &fds);
194-
return select(1, NULL, &fds, NULL, &tv) == 1;
194+
return select(1, NULL, &fds, NULL, &tv) > 0;
195195
}
196196

197197
static int socketError(SOCKET s)

0 commit comments

Comments
 (0)