Replace blocking sleep(3) and close UDS socket on failures.

The two addressed XXX were not causing any known serious bugs on their own,
but the blocking sleep was ugly and possibly in the way of further
kid registration fixes/improvements.

diff --git a/src/ipc/UdsOp.cc b/src/ipc/UdsOp.cc
index 430cfcdd3c2e51a8321fa07a3cb13da474045c27..e997965350a5b8ce3928c09ca0ccda9d427a2828 100644 (file)
--- a/src/ipc/UdsOp.cc
+++ b/src/ipc/UdsOp.cc
@@ -81,11 +81,21 @@ Ipc::UdsSender::UdsSender(const String& pathAddr, const TypedMsgHdr& aMessage):
         message(aMessage),
         retries(10), // TODO: make configurable?
         timeout(10), // TODO: make configurable?
+        sleeping(false),
         writing(false)
 {
     message.address(address);
 }
 
+void Ipc::UdsSender::swanSong()
+{
+    // did we abort while waiting between retries?
+    if (sleeping)
+        cancelSleep();
+
+    UdsOp::swanSong();
+}
+
 void Ipc::UdsSender::start()
 {
     UdsOp::start();
@@ -96,7 +106,7 @@ void Ipc::UdsSender::start()
 
 bool Ipc::UdsSender::doneAll() const
 {
-    return !writing && UdsOp::doneAll();
+    return !writing && !sleeping && UdsOp::doneAll();
 }
 
 void Ipc::UdsSender::write()
@@ -114,8 +124,53 @@ void Ipc::UdsSender::wrote(const CommIoCbParams& params)
     debugs(54, 5, HERE << params.conn << " flag " << params.flag << " retries " << retries << " [" << this << ']');
     writing = false;
     if (params.flag != COMM_OK && retries-- > 0) {
-        sleep(1); // do not spend all tries at once; XXX: use an async timed event instead of blocking here; store the time when we started writing so that we do not sleep if not needed?
-        write(); // XXX: should we close on error so that conn() reopens?
+        // perhaps a fresh connection and more time will help?
+        conn()->close();
+        sleep();
+    }
+}
+
+/// pause for a while before resending the message
+void Ipc::UdsSender::sleep()
+{
+    Must(!sleeping);
+    sleeping = true;
+    eventAdd("Ipc::UdsSender::DelayedRetry",
+             Ipc::UdsSender::DelayedRetry,
+             new Pointer(this), 1, 0, false); // TODO: Use Fibonacci increments
+}
+
+/// stop sleeping (or do nothing if we were not)
+void Ipc::UdsSender::cancelSleep()
+{
+    if (sleeping) {
+        // Why not delete the event? See Comm::ConnOpener::cancelSleep().
+        sleeping = false;
+        debugs(54, 9, "stops sleeping");
+    }
+}
+
+/// legacy wrapper for Ipc::UdsSender::delayedRetry()
+void Ipc::UdsSender::DelayedRetry(void *data)
+{
+    Pointer *ptr = static_cast<Pointer*>(data);
+    assert(ptr);
+    if (UdsSender *us = dynamic_cast<UdsSender*>(ptr->valid())) {
+        // get back inside AsyncJob protection by scheduling an async job call
+        typedef NullaryMemFunT<Ipc::UdsSender> Dialer;
+        AsyncCall::Pointer call = JobCallback(54, 4, Dialer, us, Ipc::UdsSender::delayedRetry);
+        ScheduleCallHere(call);
+    }
+    delete ptr;
+}
+
+/// make another sending attempt after a pause
+void Ipc::UdsSender::delayedRetry()
+{
+    debugs(54, 5, HERE << sleeping);
+    if (sleeping) {
+        sleeping = false;
+        write(); // reopens the connection if needed
     }
 }
 

diff --git a/src/ipc/UdsOp.h b/src/ipc/UdsOp.h
index 9c3b1eed136e07044e2c79b70fc2b8cafb811e38..d7e704799396b190513ee0af57b39851d171ef35 100644 (file)
--- a/src/ipc/UdsOp.h
+++ b/src/ipc/UdsOp.h
@@ -65,11 +65,17 @@ public:
     UdsSender(const String& pathAddr, const TypedMsgHdr& aMessage);
 
 protected:
+    virtual void swanSong(); // UdsOp (AsyncJob) API
     virtual void start(); // UdsOp (AsyncJob) API
     virtual bool doneAll() const; // UdsOp (AsyncJob) API
     virtual void timedout(); // UdsOp API
 
 private:
+    void sleep();
+    void cancelSleep();
+    static void DelayedRetry(void *data);
+    void delayedRetry();
+
     void write(); ///< schedule writing
     void wrote(const CommIoCbParams& params); ///< done writing or error
 
@@ -77,6 +83,7 @@ private:
     TypedMsgHdr message; ///< what to send
     int retries; ///< how many times to try after a write error
     int timeout; ///< total time to send the message
+    bool sleeping; ///< whether we are waiting to retry a failed write
     bool writing; ///< whether Comm started and did not finish writing
 
 private: