Avoid segfaults when scheduling an async call for a non-existent job.

Scheduling a call for invalidated or non-existent job should be a no-op
because (a) it simplifies the code and (b) the job may disappear while
the call is queued so checking job status at the call time is
pointless from the correctness point of view.

Existing code already relies on this guarantee, but apparently not too much.

The bug was in a JobDialer. I have not yet verified whether other dialers
need a similar fix.

diff --git a/src/ICAP/AsyncJob.cc b/src/ICAP/AsyncJob.cc
index d70eb77b2d83be0cd88513839b0907b8e97773db..3f7b7439b79024d6db02350e8f98f457d994acd6 100644 (file)
--- a/src/ICAP/AsyncJob.cc
+++ b/src/ICAP/AsyncJob.cc
@@ -165,34 +165,47 @@ const char *AsyncJob::status() const
 
 /* JobDialer */
 
-JobDialer::JobDialer(AsyncJob *aJob): job(aJob), lock(NULL)
+JobDialer::JobDialer(AsyncJob *aJob): job(NULL), lock(NULL)
 {
-    lock = cbdataReference(job->toCbdata());
+    if (aJob) {
+        lock = cbdataReference(aJob->toCbdata());
+        job = aJob;
+    }
 }
 
 JobDialer::JobDialer(const JobDialer &d): CallDialer(d),
-    job(d.job), lock(cbdataReference(d.lock))
+    job(NULL), lock(NULL)
 {
+    if (d.lock && cbdataReferenceValid(d.lock)) {
+        lock = cbdataReference(d.lock);
+        Must(d.job);
+        job = d.job;
+    }
 }
 
 JobDialer::~JobDialer(){
-    cbdataReferenceDone(lock);
+    cbdataReferenceDone(lock); // lock may be NULL
 }
 
 
 bool
 JobDialer::canDial(AsyncCall &call)
 {
+    if (!lock)
+        return call.cancel("job was gone before the call");
+
     if (!cbdataReferenceValid(lock))
-        return call.cancel("job is gone");
+        return call.cancel("job gone after the call");
 
+    Must(job);
     return job->canBeCalled(call);
 }
 
 void
 JobDialer::dial(AsyncCall &call) 
 {
-    assert(cbdataReferenceValid(lock)); // canDial() checks for this
+    Must(lock && cbdataReferenceValid(lock)); // canDial() checks for this
+    Must(job);
 
     job->callStart(call);