Fixes excessive closes in accept()
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 22 Jul 2014 04:55:21 +0000 (21:55 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 22 Jul 2014 04:55:21 +0000 (21:55 -0700)
The extra close brought in at 1a7a2a40, when fixing up the sockets code.
_sock_data() closes whatever ctlfd it is passed.

Incidentally, that was closing it too many times, though that bug wasn't
the one causing all of our problems.

The kernel now spits out a warning if close fails, since nobody checks
its retval.  If you see that somewhere, it's time to start asserting
your closes succeed.

kern/src/syscall.c
user/bsd/accept.c
user/bsd/socket.c

index d0ad908..f9f999c 100644 (file)
@@ -1179,6 +1179,11 @@ static intreg_t sys_close(struct proc *p, int fd)
        }
        /* 9ns, should also handle errors (bad FD, etc) */
        retval = sysclose(fd);
        }
        /* 9ns, should also handle errors (bad FD, etc) */
        retval = sysclose(fd);
+       if (retval < 0) {
+               /* no one checks their retvals.  a double close will cause problems. */
+               printk("[kernel] sys_close failed: proc %d fd %d.  Check your rets.\n",
+                      p, fd);
+       }
        return retval;
 }
 
        return retval;
 }
 
index f64efed..aa0737d 100644 (file)
@@ -65,13 +65,13 @@ accept(int fd, __SOCKADDR_ARG __addr,
                if (lcfd < 0)
                        return -1;
                /* at this point, we have a new conversation, and lcfd is its ctl fd.
                if (lcfd < 0)
                        return -1;
                /* at this point, we have a new conversation, and lcfd is its ctl fd.
-                * nfd will be the FD for that conv's data file. */
+                * nfd will be the FD for that conv's data file.  sock_data will trade
+                * our lcfd for the data file fd.  even if it fails, sock_data will
+                * close our lcfd for us.  when it succeeds, it'll open the data file
+                * before closing lcfd, which will keep the converstation alive. */
                nfd = _sock_data(lcfd, net, r->domain, r->stype, r->protocol, &nr);
                if (nfd < 0)
                        return -1;
                nfd = _sock_data(lcfd, net, r->domain, r->stype, r->protocol, &nr);
                if (nfd < 0)
                        return -1;
-               /* we have the data file, and can reopen the ctl based on the info in
-                * the rock (nr) */
-               close(lcfd);
 
                /* get remote address */
                ip = (struct sockaddr_in*)&nr->raddr;
 
                /* get remote address */
                ip = (struct sockaddr_in*)&nr->raddr;
index 7c2294d..dd6e4bd 100644 (file)
@@ -70,7 +70,7 @@ _sock_newrock(int fd)
 }
 
 /* For a ctlfd and a few other settings, it opens and returns the corresponding
 }
 
 /* For a ctlfd and a few other settings, it opens and returns the corresponding
- * datafd. */
+ * datafd.  This will close cfd for you. */
 int
 _sock_data(int cfd, char *net, int domain, int stype, int protocol, Rock **rp)
 {
 int
 _sock_data(int cfd, char *net, int domain, int stype, int protocol, Rock **rp)
 {
@@ -91,9 +91,8 @@ _sock_data(int cfd, char *net, int domain, int stype, int protocol, Rock **rp)
 
        /* open data file */
        fd = open(name, O_RDWR);
 
        /* open data file */
        fd = open(name, O_RDWR);
-       close(cfd);
+       close(cfd); /* close this no matter what */
        if(fd < 0){
        if(fd < 0){
-               close(cfd);
                errno = ENOBUFS;
                return -1;
        }
                errno = ENOBUFS;
                return -1;
        }