Cleanup: comm Timeout API

* accept Comm::Connection when setting timeout
 - use commSetConnTimeout() now

* drop old cbdata wrapper function

* drop raw-FD timeout setting (unused)

diff --git a/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc b/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
index 2f271b336e72fb142315c52e1a647d441289aea6..b509722ae0ad94ed08d0b8e055fc9639a3ed766e 100644 (file)
--- a/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
+++ b/src/DiskIO/DiskDaemon/DiskdIOStrategy.cc
@@ -210,7 +210,7 @@ DiskdIOStrategy::init()
 
     fd_note(wfd, "squid -> diskd");
 
-    commSetTimeout(wfd, -1, NULL, NULL);
+    commUnsetFdTimeout(wfd);
     commSetNonBlocking(wfd);
     Comm::QuickPollRequired();
 }

diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 4a13a596c34991cd3af072d088ed291aea84eac3..6148426ae619c7f6c2a68b8e64637ed6fffa1d3d 100644 (file)
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -234,8 +234,7 @@ void Adaptation::Icap::Xaction::noteCommConnected(const CommConnectCbParams &io)
     typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommTimeoutCbParams> TimeoutDialer;
     AsyncCall::Pointer timeoutCall =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommTimedout",
                                       TimeoutDialer(this,&Adaptation::Icap::Xaction::noteCommTimedout));
-    commSetTimeout(io.conn->fd, TheConfig.connect_timeout(
-                       service().cfg().bypass), timeoutCall);
+    commSetConnTimeout(io.conn, TheConfig.connect_timeout(service().cfg().bypass), timeoutCall);
 
     typedef CommCbMemFunT<Adaptation::Icap::Xaction, CommCloseCbParams> CloseDialer;
     closer =  asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommClosed",

diff --git a/src/comm.cc b/src/comm.cc
index 881a3c8a696a7b53ef09035b03ba54f26a7f463f..e69b5314b506dfc08f1c30ab726c9a77d6f371ea 100644 (file)
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -728,45 +728,19 @@ comm_import_opened(const Comm::ConnectionPointer &conn,
      */
 }
 
-// Legacy pre-AsyncCalls API for FD timeouts.
-int
-commSetTimeout(int fd, int timeout, CTCB * handler, void *data)
-{
-    AsyncCall::Pointer call;
-    debugs(5, 3, HERE << "FD " << fd << " timeout " << timeout);
-    if (handler != NULL)
-        call=commCbCall(5,4, "SomeTimeoutHandler", CommTimeoutCbPtrFun(handler, data));
-    else
-        call = NULL;
-    return commSetTimeout(fd, timeout, call);
-}
-
-// Legacy pre-Comm::Connection API for FD timeouts
-// still used by non-socket FD code dealing with pipes and IPC sockets.
-int
-commSetTimeout(int fd, int timeout, AsyncCall::Pointer &callback)
+// XXX: now that raw-FD timeouts are only unset for pipes and files this SHOULD be a no-op.
+// With handler already unset. Leaving this present until that can be verified for all code paths.
+void
+commUnsetFdTimeout(int fd)
 {
-    debugs(5, 3, HERE << "FD " << fd << " timeout " << timeout);
+    debugs(5, 3, HERE << "Remove timeout for FD " << fd);
     assert(fd >= 0);
     assert(fd < Squid_MaxFD);
     fde *F = &fd_table[fd];
     assert(F->flags.open);
 
-    if (timeout < 0) {
-        F->timeoutHandler = NULL;
-        F->timeout = 0;
-    } else {
-        if (callback != NULL) {
-            typedef CommTimeoutCbParams Params;
-            Params &params = GetCommParams<Params>(callback);
-            params.fd = fd;
-            F->timeoutHandler = callback;
-        }
-
-        F->timeout = squid_curtime + (time_t) timeout;
-    }
-
-    return F->timeout;
+    F->timeoutHandler = NULL;
+    F->timeout = 0;
 }
 
 int
@@ -987,10 +961,10 @@ commLingerClose(int fd, void *unused)
 }
 
 static void
-commLingerTimeout(int fd, void *unused)
+commLingerTimeout(const FdeCbParams &params)
 {
-    debugs(5, 3, "commLingerTimeout: FD " << fd);
-    comm_close(fd);
+    debugs(5, 3, "commLingerTimeout: FD " << params.fd);
+    comm_close(params.fd);
 }
 
 /*
@@ -1010,7 +984,18 @@ comm_lingering_close(int fd)
     }
 
     fd_note(fd, "lingering close");
-    commSetTimeout(fd, 10, commLingerTimeout, NULL);
+    AsyncCall::Pointer call = commCbCall(5,4, "commLingerTimeout", FdeCbPtrFun(commLingerTimeout, NULL));
+
+    debugs(5, 3, HERE << "FD " << fd << " timeout " << timeout);
+    assert(fd_table[fd].flags.open);
+    if (callback != NULL) {
+        typedef FdeCbParams Params;
+        Params &params = GetCommParams<Params>(callback);
+        params.fd = fd;
+        fd_table[fd].timeoutHandler = callback;
+        fd_table[fd].timeout = squid_curtime + static_cast<time_t>(10);
+    }
+
     Comm::SetSelect(fd, COMM_SELECT_READ, commLingerClose, NULL, 0);
 }
 
@@ -1131,7 +1116,7 @@ _comm_close(int fd, char const *file, int line)
     // a half-closed fd may lack a reader, so we stop monitoring explicitly
     if (commHasHalfClosedMonitor(fd))
         commStopHalfClosedMonitor(fd);
-    commSetTimeout(fd, -1, NULL, NULL);
+    commUnsetFdTimeout(fd);
 
     // notify read/write handlers after canceling select reservations, if any
     if (COMMIO_FD_WRITECB(fd)->active()) {

diff --git a/src/comm.h b/src/comm.h
index 5e79dd486aabe34e385d065f1f05b8f310655150..34c4461b9bdf0bc136bde73dec03ccc502eee361 100644 (file)
--- a/src/comm.h
+++ b/src/comm.h
@@ -56,8 +56,9 @@ SQUIDCEXTERN unsigned short comm_local_port(int fd);
 
 SQUIDCEXTERN int comm_udp_sendto(int sock, const Ip::Address &to, const void *buf, int buflen);
 SQUIDCEXTERN void commCallCloseHandlers(int fd);
-SQUIDCEXTERN int commSetTimeout(int fd, int, CTCB *, void *);
-extern int commSetTimeout(int fd, int, AsyncCall::Pointer &callback);
+
+/// clear a timeout handler by FD number
+extern void commUnsetFdTimeout(int fd);
 
 /**
  * Set or clear the timeout for some action on an active connection.

diff --git a/src/icmp/IcmpSquid.cc b/src/icmp/IcmpSquid.cc
index 125867f7155c3e49b8b6bb0728959a9ccd0f839d..9d46004bd6c2b3f25ff6bc34a94ce28136c76295 100644 (file)
--- a/src/icmp/IcmpSquid.cc
+++ b/src/icmp/IcmpSquid.cc
@@ -250,7 +250,7 @@ IcmpSquid::Open(void)
 
     Comm::SetSelect(icmp_sock, COMM_SELECT_READ, icmpSquidRecv, NULL, 0);
 
-    commSetTimeout(icmp_sock, -1, NULL, NULL);
+    commUnsetFdTimeout(icmp_sock);
 
     debugs(37, 1, HERE << "Pinger socket opened on FD " << icmp_sock);
 

diff --git a/src/ipc.cc b/src/ipc.cc
index 763050fc1b7fda00a10bccac28a8ccd64b45949a..4883131f15f164a6fbb3a42bd7dd18be9dac294a 100644 (file)
--- a/src/ipc.cc
+++ b/src/ipc.cc
@@ -268,7 +268,7 @@ ipcCreate(int type, const char *prog, const char *const args[], const char *name
             return ipcCloseAllFD(prfd, pwfd, crfd, cwfd);
         }
 
-        commSetTimeout(prfd, -1, NULL, NULL);
+        commUnsetFdTimeout(prfd);
         commSetNonBlocking(prfd);
         commSetNonBlocking(pwfd);
 

diff --git a/src/ipc_win32.cc b/src/ipc_win32.cc
index 4665d7ffa73ad1e9eb3dba96b19a8fa6c335333c..9a72f01474f1e2d104bd0cfe6bf97c0406d5c3dc 100644 (file)
--- a/src/ipc_win32.cc
+++ b/src/ipc_win32.cc
@@ -303,7 +303,7 @@ ipcCreate(int type, const char *prog, const char *const args[], const char *name
 
     hello_buf[x] = '\0';
     pid = atol(hello_buf);
-    commSetTimeout(prfd, -1, NULL, NULL);
+    commUnsetFdTimeout(prfd);
     commSetNonBlocking(prfd);
     commSetNonBlocking(pwfd);
     commSetCloseOnExec(prfd);

diff --git a/src/tests/stub_comm.cc b/src/tests/stub_comm.cc
index 07031e57a135c36e64c5fbba193aa3ba314731a8..25b90c364f8aa5992d497bd85984226106942852 100644 (file)
--- a/src/tests/stub_comm.cc
+++ b/src/tests/stub_comm.cc
@@ -112,11 +112,10 @@ ignoreErrno(int ierrno)
     return -1;
 }
 
-int
-commSetTimeout(int fd, int timeout, PF * handler, void *data)
+void
+commUnsetFdTimeout(int fd)
 {
     fatal ("Not implemented");
-    return -1;
 }
 
 int

diff --git a/src/unlinkd.cc b/src/unlinkd.cc
index f79d9634b840bf69116307021898db9519239fbf..5779d3b41ca3f887af46c744d0b22f5e35a8624e 100644 (file)
--- a/src/unlinkd.cc
+++ b/src/unlinkd.cc
@@ -243,9 +243,8 @@ unlinkdInit(void)
 
     fd_note(unlinkd_rfd, "unlinkd -> squid");
 
-    commSetTimeout(unlinkd_rfd, -1, NULL, NULL);
-
-    commSetTimeout(unlinkd_wfd, -1, NULL, NULL);
+    commUnsetFdTimeout(unlinkd_rfd);
+    commUnsetFdTimeout(unlinkd_wfd);
 
     /*
     * unlinkd_rfd should already be non-blocking because of