Discussion:
Missing EOF for socketpair() with SOCK_DGRAM
(too old to reply)
Christian Biere
2006-11-01 05:22:54 UTC
Permalink
Hi,

the following program does not terminate i.e., recv() never returns although
the child exits thus closing the other side of the socket pair.

#include <sys/socket.h>
#include <stdlib.h>
#include <unistd.h>
#include <err.h>

int
main(void)
{
int fd_pair[2] = { -1, -1 };
pid_t pid;

if (-1 == socketpair(AF_LOCAL, SOCK_DGRAM, 0, fd_pair))
err(EXIT_FAILURE, "socketpair");

pid = fork();
if ((pid_t) -1 == pid)
err(EXIT_FAILURE, "fork");

if (0 == pid) {
close(fd_pair[0]);
_exit(EXIT_SUCCESS);
} else {
char c;

close(fd_pair[1]);
recv(fd_pair[0], &c, sizeof c, 0);
}

return EXIT_SUCCESS;
}

For what it's worth, on FreeBSD the program terminates, on Linux recv() doesn't
return either. What's also interesting is that with some minor variations both
processes actually terminate on NetBSD as well as Linux. For example, if I
replace '0 == pid' with '0 != pid', or if I insert "poll(NULL, 0, 1000);"
before recv().

The following fixes the problem here so that recv() returns with ECONNRESET. Are
there any objections or may I commit this?

Index: uipc_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.93
diff -u -p -r1.93 uipc_usrreq.c
--- uipc_usrreq.c 3 Sep 2006 21:15:29 -0000 1.93
+++ uipc_usrreq.c 1 Nov 2006 04:14:26 -0000
@@ -799,7 +800,7 @@ unp_disconnect(struct unpcb *unp)
unp2->unp_nextref = unp->unp_nextref;
}
unp->unp_nextref = 0;
- unp->unp_socket->so_state &= ~SS_ISCONNECTED;
+ soisdisconnected(unp->unp_socket);
break;

case SOCK_STREAM:
--
Christian

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Martin Husemann
2006-11-01 08:55:15 UTC
Permalink
Post by Christian Biere
--- uipc_usrreq.c 3 Sep 2006 21:15:29 -0000 1.93
+++ uipc_usrreq.c 1 Nov 2006 04:14:26 -0000
@@ -799,7 +800,7 @@ unp_disconnect(struct unpcb *unp)
unp2->unp_nextref = unp->unp_nextref;
}
unp->unp_nextref = 0;
- unp->unp_socket->so_state &= ~SS_ISCONNECTED;
+ soisdisconnected(unp->unp_socket);
break;
This change looks correct to me.

Btw: there is a similar issue in netinet/udp_usrreq.c - conveniently marked
/* XXX */ already. Same for netinet6/raw_ip6.c and upd6_usrreq.c.

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Matt Thomas
2006-11-01 17:51:10 UTC
Permalink
Post by Martin Husemann
Post by Christian Biere
--- uipc_usrreq.c 3 Sep 2006 21:15:29 -0000 1.93
+++ uipc_usrreq.c 1 Nov 2006 04:14:26 -0000
@@ -799,7 +800,7 @@ unp_disconnect(struct unpcb *unp)
unp2->unp_nextref = unp->unp_nextref;
}
unp->unp_nextref = 0;
- unp->unp_socket->so_state &= ~SS_ISCONNECTED;
+ soisdisconnected(unp->unp_socket);
break;
This change looks correct to me.
Btw: there is a similar issue in netinet/udp_usrreq.c -
conveniently marked
/* XXX */ already. Same for netinet6/raw_ip6.c and upd6_usrreq.c.
Actually, UDP doesn't have this problem. AF_LOCAL/socketpair have
this problem
because they are a pair intimately known only to each other. That
isn't true
for UDP.

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
der Mouse
2006-11-02 02:20:13 UTC
Permalink
Post by Matt Thomas
Post by Martin Husemann
Post by Christian Biere
- unp->unp_socket->so_state &= ~SS_ISCONNECTED;
+ soisdisconnected(unp->unp_socket);
This change looks correct to me.
It looks correct to me at first sight, but I think it's wrong.

In particular, soisdisconnected() sets SS_CANTRCVMORE and
SS_CANTSENDMORE. For a socket type which can disconnect and then
reconnect (to the same or another socket), this is wrong, since
reconnecting doesn't clear SS_CANTRCVMORE and SS_CANTSENDMORE as far as
I can see (certainly soisconnected() doesn't).

If you just want the recv-breakout semantics (which I'm not convinced
are right either, though I think that's a lot more defensible), I'd
suggest trying adding the wakeup()/sowwakeup()/sorwakeup() calls from
soisdisconnected() to the SOCK_DGRAM case of unp_disconnect().
Post by Matt Thomas
Post by Martin Husemann
Btw: there is a similar issue in netinet/udp_usrreq.c - conveniently
marked /* XXX */ already.
Since one of the things there is a commented-out soisdisconnected()
call, I wouldn't want to change that without knowing why the author of
that code didn't use soisdisconnected(). (Without that, there's some
chance the author didn't know about it or didn't think of it.) My
guess would be that it's because of the SS_CANT{RCV,SEND}MORE issue.
Post by Matt Thomas
Actually, UDP doesn't have this problem. AF_LOCAL/socketpair have
this problem because they are a pair intimately known only to each
other.
Yes...but there is nothing in this code change which makes it work only
for socketpair-created sockets; it applies, as far as I can tell, to
any connected AF_LOCAL/SOCK_DGRAM socket.

/~\ The ASCII der Mouse
\ / Ribbon Campaign
X Against HTML ***@rodents.montreal.qc.ca
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...