[prev in list] [next in list] [prev in thread] [next in thread] 

List:       squid-dev
Subject:    [PATCH] AsyncCall and CommCalls changes for cleanup-comm work.
From:       Amos Jeffries <squid3 () treenet ! co ! nz>
Date:       2010-09-23 16:05:09
Message-ID: 4C9B7AB5.8050709 () treenet ! co ! nz
[Download RAW message or body]

Surprisingly small. These are the changes to src/base and CommCalls.* 
where all the Call related code has been changed.

Including the generalized Subscription mechanism used by ConnAcceptor.

CommCalls.* is still a work in progress. I'm confidant that much more of 
the code there can be removed as the cleanup progresses in the other 
components to make them use AsyncCalls more generically.

The changes visible there are indicative of most of the changes to the 
wider squid code.


Alex: removing the XXX marked const problems and testing they will work 
is blocked behind some build errors from the IOCB definition change. As 
soon as I have building code that wont hide const side-effects I'll be 
removing that muckup.

Amos
-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2

["20100923_CommCalls.patch" (text/x-patch)]

=== modified file 'src/CommCalls.cc'
--- src/CommCalls.cc	2009-07-12 22:56:47 +0000
+++ src/CommCalls.cc	2010-09-23 15:31:27 +0000
@@ -1,5 +1,6 @@
 #include "squid.h"
 #include "fde.h"
+#include "comm/Connection.h"
 #include "CommCalls.h"
 
 /* CommCommonCbParams */
@@ -22,7 +23,11 @@
 void
 CommCommonCbParams::print(std::ostream &os) const
 {
-    os << "FD " << fd;
+    if (conn != NULL)
+        os << conn;
+    else
+        os << "FD " << fd;
+
     if (xerrno)
         os << ", errno=" << xerrno;
     if (flag != COMM_OK)
@@ -34,17 +39,9 @@
 
 /* CommAcceptCbParams */
 
-CommAcceptCbParams::CommAcceptCbParams(void *aData): CommCommonCbParams(aData),
-        nfd(-1)
-{
-}
-
-void
-CommAcceptCbParams::print(std::ostream &os) const
-{
-    CommCommonCbParams::print(os);
-    if (nfd >= 0)
-        os << ", newFD " << nfd;
+CommAcceptCbParams::CommAcceptCbParams(void *aData):
+        CommCommonCbParams(aData)
+{
 }
 
 
@@ -61,19 +58,12 @@
     // drop the call if the call was scheduled before comm_close but
     // is being fired after comm_close
     if (fd >= 0 && fd_table[fd].closing()) {
-        debugs(5, 3, HERE << "droppin late connect call: FD " << fd);
+        debugs(5, 3, HERE << "dropping late connect call: FD " << fd);
         return false;
     }
     return true; // now we are in sync and can handle the call
 }
 
-void
-CommConnectCbParams::print(std::ostream &os) const
-{
-    CommCommonCbParams::print(os);
-    os << ", " << dns;
-}
-
 /* CommIoCbParams */
 
 CommIoCbParams::CommIoCbParams(void *aData): CommCommonCbParams(aData),
@@ -84,10 +74,18 @@
 bool
 CommIoCbParams::syncWithComm()
 {
+    // transition only: read/write legacy code does not know about conn, it just \
sets FD +    if (fd >= 0) {
+        if (conn == NULL)
+            conn = new Comm::Connection;
+        if (conn->fd != fd)
+            conn->fd = fd;
+    }
+
     // change parameters if the call was scheduled before comm_close but
     // is being fired after comm_close
-    if (fd >= 0 && fd_table[fd].closing() && flag != COMM_ERR_CLOSING) {
-        debugs(5, 3, HERE << "converting late call to COMM_ERR_CLOSING: FD " << fd);
+    if (conn->fd >= 0 && fd_table[conn->fd].closing() && flag != COMM_ERR_CLOSING) {
+        debugs(5, 3, HERE << "converting late call to COMM_ERR_CLOSING: " << conn);
         flag = COMM_ERR_CLOSING;
         size = 0;
     }
@@ -120,7 +118,6 @@
 {
 }
 
-
 /* CommAcceptCbPtrFun */
 
 CommAcceptCbPtrFun::CommAcceptCbPtrFun(IOACB *aHandler,
@@ -130,10 +127,16 @@
 {
 }
 
+CommAcceptCbPtrFun::CommAcceptCbPtrFun(const CommAcceptCbPtrFun &o):
+        CommDialerParamsT<CommAcceptCbParams>(o.params),
+        handler(o.handler)
+{
+}
+
 void
 CommAcceptCbPtrFun::dial()
 {
-    handler(params.fd, params.nfd, &params.details, params.flag, params.xerrno, \
params.data); +    handler(params.fd, params.conn, params.flag, params.xerrno, \
params.data);  }
 
 void
@@ -157,7 +160,7 @@
 void
 CommConnectCbPtrFun::dial()
 {
-    handler(params.fd, params.dns, params.flag, params.xerrno, params.data);
+    handler(params.conn, params.flag, params.xerrno, params.data);
 }
 
 void
@@ -180,7 +183,7 @@
 void
 CommIoCbPtrFun::dial()
 {
-    handler(params.fd, params.buf, params.size, params.flag, params.xerrno, \
params.data); +    handler(params.conn, params.buf, params.size, params.flag, \
params.xerrno, params.data);  }
 
 void
@@ -227,6 +230,16 @@
 void
 CommTimeoutCbPtrFun::dial()
 {
+    // AYJ NP: since the old code is still used by pipes and IPC
+    // we cant discard the params.fd functions entirely for old callbacks.
+    // new callers supposed to only set conn.
+    // sync FD and conn fields at this single failure point before dialing.
+    if (params.conn != NULL) {
+        if (params.fd < 0 && params.conn->fd > 0)
+            params.fd = params.conn->fd;
+        assert(params.fd == params.conn->fd); // Must() ?
+    }
+
     handler(params.fd, params.data);
 }
 

=== modified file 'src/CommCalls.h'
--- src/CommCalls.h	2010-08-24 00:12:54 +0000
+++ src/CommCalls.h	2010-09-22 13:29:30 +0000
@@ -6,11 +6,10 @@
 #ifndef SQUID_COMMCALLS_H
 #define SQUID_COMMCALLS_H
 
-#include "comm.h"
-#include "ConnectionDetail.h"
-#include "DnsLookupDetails.h"
 #include "base/AsyncCall.h"
 #include "base/AsyncJobCalls.h"
+#include "comm/comm_err_t.h"
+#include "comm/forward.h"
 
 /* CommCalls implement AsyncCall interface for comm_* callbacks.
  * The classes cover two call dialer kinds:
@@ -22,6 +21,10 @@
  *     - I/O (IOCB).
  */
 
+typedef void IOACB(int fd, const Comm::ConnectionPointer &details, comm_err_t flag, \
int xerrno, void *data); +typedef void CNCB(const Comm::ConnectionPointer &conn, \
comm_err_t status, int xerrno, void *data); +typedef void IOCB(const \
Comm::ConnectionPointer &conn, char *, size_t size, comm_err_t flag, int xerrno, void \
*data); +
 /*
  * TODO: When there are no function-pointer-based callbacks left, all
  * this complexity can be removed. Jobs that need comm services will just
@@ -51,7 +54,8 @@
 
 public:
     void *data; // cbdata-protected
-    int fd;
+    Comm::ConnectionPointer conn;
+    int fd; // raw FD from legacy calls. use conn instead.
     int xerrno;
     comm_err_t flag;
 
@@ -65,12 +69,6 @@
 {
 public:
     CommAcceptCbParams(void *aData);
-
-    void print(std::ostream &os) const;
-
-public:
-    ConnectionDetail details;
-    int nfd; // TODO: rename to fdNew or somesuch
 };
 
 // connect parameters
@@ -80,11 +78,6 @@
     CommConnectCbParams(void *aData);
 
     bool syncWithComm(); // see CommCommonCbParams::syncWithComm
-
-    void print(std::ostream &os) const;
-
-public:
-    DnsLookupDetails dns;
 };
 
 // read/write (I/O) parameters
@@ -176,10 +169,12 @@
 {
 public:
     typedef CommAcceptCbParams Params;
+    typedef RefCount<CommAcceptCbPtrFun> Pointer;
 
     CommAcceptCbPtrFun(IOACB *aHandler, const CommAcceptCbParams &aParams);
+    CommAcceptCbPtrFun(const CommAcceptCbPtrFun &o);
+
     void dial();
-
     virtual void print(std::ostream &os) const;
 
 public:
@@ -259,11 +254,20 @@
 class CommCbFunPtrCallT: public AsyncCall
 {
 public:
+    typedef RefCount<CommCbFunPtrCallT<Dialer> > Pointer;
     typedef typename Dialer::Params Params;
 
     inline CommCbFunPtrCallT(int debugSection, int debugLevel,
                              const char *callName, const Dialer &aDialer);
 
+// XXX: obsolete comment?
+    // parameter cannot be const because getDialer() cannot be const
+    // getDialer() cannot because Comm IO syncWithComm() alters the object params \
data +    inline CommCbFunPtrCallT(const Pointer &p) :
+            AsyncCall(p->debugSection, p->debugLevel, p->name),
+            dialer(p->dialer)
+        {}
+
     virtual CallDialer* getDialer() { return &dialer; }
 
 public:

=== modified file 'src/base/AsyncCall.cc'
--- src/base/AsyncCall.cc	2009-04-09 22:31:13 +0000
+++ src/base/AsyncCall.cc	2010-09-10 12:36:02 +0000
@@ -77,4 +77,3 @@
     AsyncCallQueue::Instance().schedule(call);
     return true;
 }
-

=== modified file 'src/base/AsyncCall.h'
--- src/base/AsyncCall.h	2009-04-09 22:31:13 +0000
+++ src/base/AsyncCall.h	2010-09-11 14:58:33 +0000
@@ -44,6 +44,8 @@
     friend class AsyncCallQueue;
 
     AsyncCall(int aDebugSection, int aDebugLevel, const char *aName);
+    AsyncCall();
+    AsyncCall(const AsyncCall &);
     virtual ~AsyncCall();
 
     void make(); // fire if we can; handles general call debugging
@@ -119,6 +121,10 @@
                const Dialer &aDialer): AsyncCall(aDebugSection, aDebugLevel, aName),
             dialer(aDialer) {}
 
+    AsyncCallT(const RefCount<AsyncCallT<Dialer> > &o):
+            AsyncCall(o->debugSection, o->debugLevel, o->name),
+            dialer(o->dialer) {}
+
     CallDialer *getDialer() { return &dialer; }
 
 protected:

=== modified file 'src/base/Makefile.am'
--- src/base/Makefile.am	2010-08-23 23:15:26 +0000
+++ src/base/Makefile.am	2010-09-10 18:00:55 +0000
@@ -13,5 +13,6 @@
 	AsyncCallQueue.cc \
 	AsyncCallQueue.h \
 	CbcPointer.h \
+	Subscription.h \
 	TextException.cc \
 	TextException.h

=== added file 'src/base/Subscription.h'
--- src/base/Subscription.h	1970-01-01 00:00:00 +0000
+++ src/base/Subscription.h	2010-09-23 15:37:02 +0000
@@ -0,0 +1,50 @@
+#ifndef _SQUID_BASE_SUBSCRIPTION_H
+#define _SQUID_BASE_SUBSCRIPTION_H
+
+#include "base/AsyncCall.h"
+
+/**
+ * API for classes needing to emit multiple event-driven AsyncCalls.
+ *
+ * The emitter class needs to accept and store a Subscription::Pointer.
+ * The callback() function will spawn AsyncCalls to be filled out and
+ * scheduled wth every event happening.
+ */
+class Subscription: public RefCountable
+{
+public:
+    typedef RefCount<Subscription> Pointer;
+
+    /// returns a call object to be used for the next call back
+    virtual AsyncCall::Pointer callback() = 0;
+//    virtual AsyncCall::Pointer callback() const = 0;
+};
+
+/**
+ * Implements Subscription API using Call's copy constructor.
+ *
+ * A receiver class allocates one of these templated from the Call type
+ * to be received (usually AsyncCallT) and passes it to the emitter class
+ * which will use it to spawn event calls.
+ *
+ * To be a subscriber the AsyncCall child must implement a copy constructor.
+ */
+template<class Call_>
+class CallSubscription: public Subscription
+{
+public:
+    CallSubscription(const RefCount<Call_> &aCall) : call(aCall) {};
+
+// XXX: obsolete comment?
+    // cant be const sometimes because CommCbFunPtrCallT cant provide a const \
overload. +    // CommCbFunPtrCallT lists why. boils down to Comm IO syncWithComm() \
existence +    // NP: we still treat it as const though.
+    CallSubscription(RefCount<Call_> &aCall) : call(aCall) {};
+    virtual AsyncCall::Pointer callback() { return new Call_(call); };
+//    virtual AsyncCall::Pointer callback() const { return new Call_(call); };
+
+private:
+    RefCount<Call_> call; ///< gets copied to create callback calls
+};
+
+#endif /* _SQUID_BASE_SUBSCRIPTION_H */



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic