From 90a4480409fe01e14ef52b8ccbfa1ab199008b5f Mon Sep 17 00:00:00 2001 From: didg Date: Sun, 25 Oct 2009 06:12:51 +0000 Subject: [PATCH] 1) try a better workaround for deadlocks when both the server and the client are sleeping in write by using a timeout. 2) Don't block signals when the server write to the socket, SIGTERM is not fully protect yet, the server can still disconnect in a middle of a write --- etc/afpd/afp_dsi.c | 23 ++++++++--- etc/afpd/fork.c | 13 +----- etc/afpd/volume.c | 16 +++++++- include/atalk/dsi.h | 10 ++--- libatalk/dsi/dsi_attn.c | 16 ++++---- libatalk/dsi/dsi_cmdreply.c | 6 +-- libatalk/dsi/dsi_init.c | 7 +--- libatalk/dsi/dsi_read.c | 28 +------------ libatalk/dsi/dsi_stream.c | 79 +++++++++++++++++++++++++------------ libatalk/dsi/dsi_tcp.c | 24 ++++++++++- libatalk/dsi/dsi_tickle.c | 14 +++---- 11 files changed, 131 insertions(+), 105 deletions(-) diff --git a/etc/afpd/afp_dsi.c b/etc/afpd/afp_dsi.c index ac6d8fbc..3a73159a 100644 --- a/etc/afpd/afp_dsi.c +++ b/etc/afpd/afp_dsi.c @@ -1,5 +1,5 @@ /* - * $Id: afp_dsi.c,v 1.45 2009-10-22 13:40:11 franklahm Exp $ + * $Id: afp_dsi.c,v 1.46 2009-10-25 06:12:51 didg Exp $ * * Copyright (c) 1999 Adrian Sun (asun@zoology.washington.edu) * Copyright (c) 1990,1993 Regents of The University of Michigan. @@ -59,6 +59,7 @@ static void afp_dsi_close(AFPObj *obj) { DSI *dsi = obj->handle; + /* XXX we have to check we are not root here */ close_all_vol(); if (obj->logout) (*obj->logout)(); @@ -115,8 +116,11 @@ static void afp_dsi_timedown(int sig _U_) child.flags |= CHILD_DIE; /* shutdown and don't reconnect. server going down in 5 minutes. */ setmessage("The server is going down for maintenance."); - dsi_attention(child.obj->handle, AFPATTN_SHUTDOWN | AFPATTN_NORECONNECT | - AFPATTN_MESG | AFPATTN_TIME(5)); + if (dsi_attention(child.obj->handle, AFPATTN_SHUTDOWN | AFPATTN_NORECONNECT | + AFPATTN_MESG | AFPATTN_TIME(5)) < 0) { + DSI *dsi = (DSI *) child.obj->handle; + dsi->down_request = 1; + } it.it_interval.tv_sec = 0; it.it_interval.tv_usec = 0; @@ -146,7 +150,6 @@ static void afp_dsi_timedown(int sig _U_) LOG(log_error, logtype_afpd, "afp_timedown: sigaction SIGHUP: %s", strerror(errno) ); afp_dsi_die(EXITERR_SYS); } - } /* --------------------------------- @@ -167,7 +170,8 @@ static void afp_dsi_getmesg (int sig _U_) DSI *dsi = (DSI *) child.obj->handle; dsi->msg_request = 1; - dsi_attention(child.obj->handle, AFPATTN_MESG | AFPATTN_TIME(5)); + if (dsi_attention(child.obj->handle, AFPATTN_MESG | AFPATTN_TIME(5)) < 0) + dsi->msg_request = 2; } #endif /* SERVERTEXT */ @@ -215,9 +219,18 @@ static void pending_request(DSI *dsi) /* read msg if any, it could be done in afp_getsrvrmesg */ if (dsi->msg_request) { + if (dsi->msg_request == 2) { + /* didn't send it in signal handler */ + dsi_attention(child.obj->handle, AFPATTN_MESG | AFPATTN_TIME(5)); + } dsi->msg_request = 0; readmessage(child.obj); } + if (dsi->down_request) { + dsi->down_request = 0; + dsi_attention(child.obj->handle, AFPATTN_SHUTDOWN | AFPATTN_NORECONNECT | + AFPATTN_MESG | AFPATTN_TIME(5)); + } } /* ------------------------------------------- diff --git a/etc/afpd/fork.c b/etc/afpd/fork.c index 57b3268e..07ce3ab1 100644 --- a/etc/afpd/fork.c +++ b/etc/afpd/fork.c @@ -1,5 +1,5 @@ /* - * $Id: fork.c,v 1.67 2009-10-22 12:35:38 franklahm Exp $ + * $Id: fork.c,v 1.68 2009-10-25 06:12:51 didg Exp $ * * Copyright (c) 1990,1993 Regents of The University of Michigan. * All Rights Reserved. See COPYRIGHT. @@ -910,7 +910,6 @@ static int read_fork(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, si if ((obj->proto == AFPPROTO_DSI) && (*rbuflen < reqcount) && !nlmask) { DSI *dsi = obj->handle; off_t size; - int non_blocking = 0; /* reqcount isn't always truthful. we need to deal with that. */ size = ad_size(ofork->of_ad, eid); @@ -951,12 +950,6 @@ static int read_fork(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, si afp_read_loop: #endif - /* fill up our buffer. */ - if (*rbuflen) { - /* set to non blocking mode */ - non_blocking = 1; - dsi_block(dsi, 1); - } /* fill up our buffer. */ while (*rbuflen > 0) { cc = read_file(ofork, eid, offset, nlmask, nlchar, rbuf,rbuflen, xlate); @@ -976,10 +969,6 @@ afp_read_loop: goto afp_read_exit; *rbuflen = cc; } - if (non_blocking) { - /* set back to blocking mode */ - dsi_block(dsi, 0); - } dsi_readdone(dsi); goto afp_read_done; diff --git a/etc/afpd/volume.c b/etc/afpd/volume.c index 47aabef8..ee13de80 100644 --- a/etc/afpd/volume.c +++ b/etc/afpd/volume.c @@ -1,5 +1,5 @@ /* - * $Id: volume.c,v 1.94 2009-10-15 10:43:13 didg Exp $ + * $Id: volume.c,v 1.95 2009-10-25 06:12:51 didg Exp $ * * Copyright (c) 1990,1993 Regents of The University of Michigan. * All Rights Reserved. See COPYRIGHT. @@ -2136,8 +2136,18 @@ struct extmap *getdefextmap(void) /* -------------------------- poll if a volume is changed by other processes. + return + 0 no attention msg sent + 1 attention msg sent + -1 error (socket closed) + + Note: if attention return -1 no packet has been + sent because the buffer is full, we don't care + either there's no reader or there's a lot of + traffic and another pollvoltime will follow */ int pollvoltime(AFPObj *obj) + { struct vol *vol; struct timeval tv; @@ -2186,7 +2196,9 @@ void setvoltime(AFPObj *obj, struct vol *vol) /* a little granularity */ if (vol->v_mtime < tv.tv_sec) { vol->v_mtime = tv.tv_sec; - /* or finder doesn't update free space */ + /* or finder doesn't update free space + * XXX is it still true with newer OSX? + */ if (afp_version > 21 && obj->options.server_notif) { obj->attention(obj->handle, AFPATTN_NOTIFY | AFPATTN_VOLCHANGED); } diff --git a/include/atalk/dsi.h b/include/atalk/dsi.h index b2e6cd98..de7beaf8 100644 --- a/include/atalk/dsi.h +++ b/include/atalk/dsi.h @@ -58,14 +58,13 @@ typedef struct DSI { struct dsi_block header; struct sockaddr_in server, client; - sigset_t sigblockset, oldset; - int sigblocked; struct itimerval timer; int in_write; /* in the middle of writing multiple packets, signal handlers * can't write to the socket */ int msg_request; /* pending message to the client */ + int down_request; /* pending SIGUSR1 down in 5 mn */ u_int32_t attn_quantum, datasize, server_quantum; u_int16_t serverID, clientID; @@ -92,7 +91,6 @@ typedef struct DSI { #endif /* buffer for OSX deadlock */ - int noblocking; char *buffer; char *start; char *eof; @@ -164,11 +162,9 @@ extern void dsi_getstatus (DSI *); extern void dsi_close (DSI *); extern void dsi_sleep (DSI *, const int ); -/* set, unset socket blocking mode */ -extern int dsi_block (DSI *, const int); - +#define DSI_NOWAIT 1 /* low-level stream commands -- in dsi_stream.c */ -extern size_t dsi_stream_write (DSI *, void *, const size_t, const int mode); +extern ssize_t dsi_stream_write (DSI *, void *, const size_t, const int mode); extern size_t dsi_stream_read (DSI *, void *, const size_t); extern int dsi_stream_send (DSI *, void *, size_t); extern int dsi_stream_receive (DSI *, void *, const size_t, size_t *); diff --git a/libatalk/dsi/dsi_attn.c b/libatalk/dsi/dsi_attn.c index 395ee1db..8a9eadde 100644 --- a/libatalk/dsi/dsi_attn.c +++ b/libatalk/dsi/dsi_attn.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_attn.c,v 1.7 2009-10-22 04:59:50 didg Exp $ + * $Id: dsi_attn.c,v 1.8 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -31,13 +31,15 @@ int dsi_attention(DSI *dsi, AFPUserBytes flags) { /* header + AFPUserBytes */ char block[DSI_BLOCKSIZ + sizeof(AFPUserBytes)]; - sigset_t oldset; u_int32_t len, nlen; u_int16_t id; - if (dsi->asleep || dsi->in_write) + if (dsi->asleep) return 1; - + + if (dsi->in_write) { + return -1; + } id = htons(dsi_serverID(dsi)); flags = htons(flags); len = MIN(sizeof(flags), dsi->attn_quantum); @@ -53,9 +55,5 @@ int dsi_attention(DSI *dsi, AFPUserBytes flags) /* reserved = 0 */ /* send an attention */ - sigprocmask(SIG_BLOCK, &dsi->sigblockset, &oldset); - len = dsi_stream_write(dsi, block, DSI_BLOCKSIZ + len, 0); - sigprocmask(SIG_SETMASK, &oldset, NULL); - - return len; + return dsi_stream_write(dsi, block, DSI_BLOCKSIZ + len, DSI_NOWAIT); } diff --git a/libatalk/dsi/dsi_cmdreply.c b/libatalk/dsi/dsi_cmdreply.c index f2223fff..f924da5a 100644 --- a/libatalk/dsi/dsi_cmdreply.c +++ b/libatalk/dsi/dsi_cmdreply.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_cmdreply.c,v 1.4 2005-04-28 20:50:02 bfernhomberg Exp $ + * $Id: dsi_cmdreply.c,v 1.5 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -25,9 +25,5 @@ int ret; dsi->header.dsi_code = htonl(err); ret = dsi_stream_send(dsi, dsi->data, dsi->datalen); - if (dsi->sigblocked) { - sigprocmask(SIG_SETMASK, &dsi->oldset, NULL); - dsi->sigblocked = 0; - } return ret; } diff --git a/libatalk/dsi/dsi_init.c b/libatalk/dsi/dsi_init.c index 6d8405fa..5ac8dec7 100644 --- a/libatalk/dsi/dsi_init.c +++ b/libatalk/dsi/dsi_init.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_init.c,v 1.8 2009-10-22 05:53:20 didg Exp $ + * $Id: dsi_init.c,v 1.9 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -28,11 +28,6 @@ DSI *dsi_init(const dsi_proto protocol, const char *program, dsi->server_quantum = quantum; /* default server quantum */ dsi->program = program; - /* signals to block. we actually disable timers for "known" - * large transfers (i.e., dsi_read/write). */ - sigemptyset(&dsi->sigblockset); - sigaddset(&dsi->sigblockset, SIGTERM); - sigaddset(&dsi->sigblockset, SIGUSR1); switch (protocol) { /* currently the only transport protocol that exists for dsi */ case DSI_TCPIP: diff --git a/libatalk/dsi/dsi_read.c b/libatalk/dsi/dsi_read.c index cd004dbc..55345ba8 100644 --- a/libatalk/dsi/dsi_read.c +++ b/libatalk/dsi/dsi_read.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_read.c,v 1.6 2009-10-22 04:59:50 didg Exp $ + * $Id: dsi_read.c,v 1.7 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -22,7 +22,6 @@ #endif #include -#include #ifndef min #define min(a,b) ((a) < (b) ? (a) : (b)) @@ -43,8 +42,6 @@ ssize_t dsi_readinit(DSI *dsi, void *buf, const size_t buflen, dsi->header.dsi_len = htonl(size); dsi->header.dsi_code = htonl(err); - sigprocmask(SIG_BLOCK, &dsi->sigblockset, &dsi->oldset); - dsi->sigblocked = 1; dsi->in_write++; if (dsi_stream_send(dsi, buf, buflen)) { dsi->datasize = size - buflen; @@ -56,36 +53,15 @@ ssize_t dsi_readinit(DSI *dsi, void *buf, const size_t buflen, void dsi_readdone(DSI *dsi) { - sigprocmask(SIG_SETMASK, &dsi->oldset, NULL); - dsi->sigblocked = 0; dsi->in_write--; } -/* */ -int dsi_block(DSI *dsi, const int mode) -{ -#if 0 - dsi->noblocking = mode; - return 0; -#else - int adr = mode; - int ret; - - ret = ioctl(dsi->socket, FIONBIO, &adr); - if (!ret) { - dsi->noblocking = mode; - } - return ret; -#endif -} - /* send off the data */ ssize_t dsi_read(DSI *dsi, void *buf, const size_t buflen) { size_t len; - int delay = (dsi->datasize != buflen)?1:0; - len = dsi_stream_write(dsi, buf, buflen, delay); + len = dsi_stream_write(dsi, buf, buflen, 0); if (len == buflen) { dsi->datasize -= len; diff --git a/libatalk/dsi/dsi_stream.c b/libatalk/dsi/dsi_stream.c index fe03d9e4..f768e31f 100644 --- a/libatalk/dsi/dsi_stream.c +++ b/libatalk/dsi/dsi_stream.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_stream.c,v 1.16 2009-10-22 13:40:11 franklahm Exp $ + * $Id: dsi_stream.c,v 1.17 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1998 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -37,6 +37,7 @@ #include #include +#include #define min(a,b) ((a) < (b) ? (a) : (b)) @@ -68,13 +69,27 @@ static void dsi_init_buffer(DSI *dsi) } } -/* ---------------------- */ -static void dsi_buffer(DSI *dsi) +/* ---------------------- + afpd is sleeping too much while trying to send something. + May be there's no reader or the reader is also sleeping in write, + look if there's some data for us to read, hopefully it will wake up + the reader +*/ +static int dsi_buffer(DSI *dsi) { fd_set readfds, writefds; int len; int maxfd; + int adr; + /* non blocking mode */ + adr = 1; + if (ioctl(dsi->socket, FIONBIO, &adr) < 0) { + /* can't do it! exit without error it will sleep to death below */ + LOG(log_error, logtype_default, "dsi_buffer: ioctl non blocking mode %s", strerror(errno)); + return 0; + } + FD_ZERO(&readfds); FD_ZERO(&writefds); FD_SET( dsi->socket, &readfds); @@ -84,11 +99,11 @@ static void dsi_buffer(DSI *dsi) FD_SET( dsi->socket, &readfds); FD_SET( dsi->socket, &writefds); if (select( maxfd, &readfds, &writefds, NULL, NULL) <= 0) - return; + break; if ( !FD_ISSET(dsi->socket, &readfds)) { /* nothing waiting in the read queue */ - return; + break; } dsi_init_buffer(dsi); len = dsi->end - dsi->eof; @@ -98,25 +113,32 @@ static void dsi_buffer(DSI *dsi) * fall back to blocking IO * could block and disconnect but it's better than a cpu hog */ - dsi_block(dsi, 0); - return; + break; } len = read(dsi->socket, dsi->eof, len); if (len <= 0) - return; + break; dsi->eof += len; if ( FD_ISSET(dsi->socket, &writefds)) { - return; + /* we can write again at last */ + break; } } + adr = 0; + if (ioctl(dsi->socket, FIONBIO, &adr) < 0) { + /* can't do it! afpd will fail very quickly */ + LOG(log_error, logtype_default, "dsi_buffer: ioctl blocking mode %s", strerror(errno)); + return -1; + } + return 0; } /* ------------------------------ * write raw data. return actual bytes read. checks against EINTR * aren't necessary if all of the signals have SA_RESTART * specified. */ -size_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode _U_) +ssize_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode) { size_t written; ssize_t len; @@ -129,9 +151,7 @@ size_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode _U_) #if 0 /* XXX there's no MSG_DONTWAIT in recv ?? so we have to play with ioctl */ - if (dsi->noblocking) { - flags |= MSG_DONTWAIT; - } + flags |= MSG_DONTWAIT; #endif dsi->in_write++; @@ -143,17 +163,23 @@ size_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode _U_) continue; if (len < 0) { - if (dsi->noblocking && errno == EAGAIN) { - /* non blocking mode but will block - * read data in input queue. - * - */ - dsi_buffer(dsi); - } - else { - LOG(log_error, logtype_default, "dsi_stream_write: %s", strerror(errno)); - break; + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (mode == DSI_NOWAIT && written == 0) { + /* DSI_NOWAIT is used by attention + give up in this case. + */ + return -1; + } + if (dsi_buffer(dsi)) { + /* can't go back to blocking mode, exit, the next read + will return with an error and afpd will die. + */ + break; + } + continue; } + LOG(log_error, logtype_default, "dsi_stream_write: %s", strerror(errno)); + break; } else { written += len; @@ -269,7 +295,6 @@ void dsi_sleep(DSI *dsi, const int state) static void block_sig(DSI *dsi) { dsi->in_write++; - if (!dsi->sigblocked) sigprocmask(SIG_BLOCK, &dsi->sigblockset, &dsi->oldset); } /* --------------------------------------- @@ -277,7 +302,6 @@ static void block_sig(DSI *dsi) static void unblock_sig(DSI *dsi) { dsi->in_write--; - if (!dsi->sigblocked) sigprocmask(SIG_SETMASK, &dsi->oldset, NULL); } /* --------------------------------------- @@ -325,6 +349,11 @@ int dsi_stream_send(DSI *dsi, void *buf, size_t length) if ((size_t)len == towrite) /* wrote everything out */ break; else if (len < 0) { /* error */ + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (!dsi_buffer(dsi)) { + continue; + } + } LOG(log_error, logtype_default, "dsi_stream_send: %s", strerror(errno)); unblock_sig(dsi); return 0; diff --git a/libatalk/dsi/dsi_tcp.c b/libatalk/dsi/dsi_tcp.c index 5f6b6eb4..4de3da23 100644 --- a/libatalk/dsi/dsi_tcp.c +++ b/libatalk/dsi/dsi_tcp.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_tcp.c,v 1.15 2009-10-13 22:55:37 didg Exp $ + * $Id: dsi_tcp.c,v 1.16 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997, 1998 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -86,6 +86,25 @@ static void dsi_tcp_close(DSI *dsi) dsi->socket = -1; } +static void dsi_tcp_timeout(DSI *dsi) +{ + struct timeval tv; + /* 2 seconds delay, most of the time it translates to 4 seconds: + * send/write returns first with whatever it has written and the + * second time it returns EAGAIN + */ + tv.tv_sec = 2; + tv.tv_usec = 0; + + /* Note: write isn't a restartable syscall if there's a timeout on the socket + * we have to test for EINTR + */ + if (setsockopt(dsi->socket, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)) < 0) { + LOG(log_error, logtype_default, "dsi_tcp_open: unable to set timeout %s", strerror(errno)); + exit(EXITERR_CLNT); + } +} + /* alarm handler for tcp_open */ static void timeout_handler(int sig _U_) { @@ -204,6 +223,9 @@ static int dsi_tcp_open(DSI *dsi) setitimer(ITIMER_REAL, &timer, NULL); sigaction(SIGALRM, &oldact, NULL); #endif + + dsi_tcp_timeout(dsi); + LOG(log_info, logtype_default,"ASIP session:%u(%d) from %s:%u(%d)", ntohs(dsi->server.sin_port), dsi->serversock, inet_ntoa(dsi->client.sin_addr), ntohs(dsi->client.sin_port), diff --git a/libatalk/dsi/dsi_tickle.c b/libatalk/dsi/dsi_tickle.c index 5b5c7e7b..02aa0536 100644 --- a/libatalk/dsi/dsi_tickle.c +++ b/libatalk/dsi/dsi_tickle.c @@ -1,5 +1,5 @@ /* - * $Id: dsi_tickle.c,v 1.7 2009-10-22 04:59:50 didg Exp $ + * $Id: dsi_tickle.c,v 1.8 2009-10-25 06:13:11 didg Exp $ * * Copyright (c) 1997 Adrian Sun (asun@zoology.washington.edu) * All rights reserved. See COPYRIGHT. @@ -18,12 +18,10 @@ #include /* server generated tickles. as this is only called by the tickle handler, - * we don't need to block signals. well, actually, we might get it during - * a SIGHUP. */ + * we don't need to block signals. */ int dsi_tickle(DSI *dsi) { char block[DSI_BLOCKSIZ]; - sigset_t oldset; u_int16_t id; int ret; @@ -38,9 +36,11 @@ int dsi_tickle(DSI *dsi) memcpy(block + 2, &id, sizeof(id)); /* code = len = reserved = 0 */ - sigprocmask(SIG_BLOCK, &dsi->sigblockset, &oldset); - ret = dsi_stream_write(dsi, block, DSI_BLOCKSIZ, 0) == DSI_BLOCKSIZ; - sigprocmask(SIG_SETMASK, &oldset, NULL); + ret = dsi_stream_write(dsi, block, DSI_BLOCKSIZ, DSI_NOWAIT); + /* we don't really care if we can't send a tickle, it will fail + * elsewhere + */ + ret = (ret == -1 || ret == DSI_BLOCKSIZ); return ret; } -- 2.39.2