*** empty log message ***

3 files changed, 210 insertions, 83 deletions

diff --git a/ace/Future.cpp b/ace/Future.cpp
index d240a9adc42..2e77d001ba4 100644
--- a/ace/Future.cpp
+++ b/ace/Future.cpp
@@ -17,8 +17,8 @@ ACE_Future_Rep<T>::dump (void) const
 {
   ACE_DEBUG ((LM_DEBUG, ACE_BEGIN_DUMP, this));
   ACE_DEBUG ((LM_DEBUG,
-	      "ref_count_ = %d\n",
-	      (int) this->ref_count_));
+              "ref_count_ = %d\n",
+              (int) this->ref_count_));
   ACE_DEBUG ((LM_INFO,"value_: \n"));
   if (this->value_)
     ACE_DEBUG ((LM_DEBUG," (NON-NULL)\n"));
@@ -36,35 +36,58 @@ template <class T> ACE_Future_Rep<T> *
 ACE_Future_Rep<T>::create (void)
 {
   // Yes set ref count to zero.
-  ACE_NEW_RETURN (((ACE_Future<T> *) this)->future_rep_, ACE_Future_Rep<T>, 0);
+  return new ACE_Future_Rep<T>();
 }
 
 template <class T> ACE_Future_Rep<T> * 
-ACE_Future_Rep<T>::attach (ACE_Future_Rep<T> *rep)
+ACE_Future_Rep<T>::attach (ACE_Future_Rep<T>*& rep)
 {
   ACE_ASSERT (rep != 0);
   // Use value_ready_mutex_ for both condition and ref count management
-  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, r_mon, (ACE_Thread_Mutex &) rep->value_ready_mutex_));
+  ACE_MT (ACE_Guard<ACE_Thread_Mutex> r_mon(rep->value_ready_mutex_));
   ++rep->ref_count_;
   return rep;
 }
 
 template <class T> void 
-ACE_Future_Rep<T>::detach (ACE_Future_Rep<T> *rep)
+ACE_Future_Rep<T>::detach (ACE_Future_Rep<T>*& rep)
 {
   ACE_ASSERT(rep != 0);
   // Use value_ready_mutex_ for both condition and ref count management
-  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, r_mon, (ACE_Thread_Mutex &) rep->value_ready_mutex_));
+  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, r_mon, rep->value_ready_mutex_));
   
   if (rep->ref_count_-- == 0)
     {
       r_mon.release ();
       // We do not need the lock when deleting the representation.
-      // There should be no side effects from deleting rep.
+      // There should be no side effects from deleting rep and we don
+      // not want to release a deleted mutex.
       delete rep;
     }
 }
 
+template <class T> void 
+ACE_Future_Rep<T>::assign (ACE_Future_Rep<T>*& rep, ACE_Future_Rep<T>* new_rep)
+{
+  ACE_ASSERT(rep != 0);
+  ACE_ASSERT(new_rep != 0);
+  // Use value_ready_mutex_ for both condition and ref count management
+  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, r_mon, rep->value_ready_mutex_));
+  
+  ACE_Future_Rep<T>* old = rep;
+  rep = new_rep;
+  
+  // detached old last for exception safety
+  if (old->ref_count_-- == 0)
+    {
+      r_mon.release ();
+      // We do not need the lock when deleting the representation.
+      // There should be no side effects from deleting rep and we don
+      // not want to release a deleted mutex.
+      delete old;
+    }
+}
+
 template <class T> 
 ACE_Future_Rep<T>::ACE_Future_Rep (void)
   : value_ (0),
@@ -96,7 +119,7 @@ ACE_Future_Rep<T>::set (const T &r)
       // Double-checked locking pattern to avoid multiple allocations.
 
       if (this->value_ == 0)
-	ACE_NEW_RETURN (this->value_, T (r), -1);
+        ACE_NEW_RETURN (this->value_, T (r), -1);
 
       // Signal all the waiting threads.
       return this->value_ready_.broadcast ();
@@ -108,7 +131,7 @@ ACE_Future_Rep<T>::set (const T &r)
 
 template <class T> int
 ACE_Future_Rep<T>::get (T &value,
-			ACE_Time_Value *tv)
+                        ACE_Time_Value *tv)
 {
   // If the value is already produced, return it.
   if (this->value_ == 0)
@@ -119,9 +142,9 @@ ACE_Future_Rep<T>::get (T &value,
       // producer writes to it.
 
       while (this->value_ == 0) 
-	// Perform a timed wait.
-	if (this->value_ready_.wait (tv) == -1)
-	  return -1;
+        // Perform a timed wait.
+        if (this->value_ready_.wait (tv) == -1)
+          return -1;
 
       // Destructor releases the lock.
     }
@@ -145,11 +168,11 @@ ACE_Future_Rep<T>::operator T ()
       // Wait ``forever.''
 
       while (this->value_ == 0) 
-	if (this->value_ready_.wait () == -1)
-	  // What to do in this case since we've got to indicate
-	  // failure somehow?  Exceptions would be nice, but they're
-	  // not portable...
-	  return 0; 
+        if (this->value_ready_.wait () == -1)
+          // What to do in this case since we've got to indicate
+          // failure somehow?  Exceptions would be nice, but they're
+          // not portable...
+          return 0; 
 
       // Destructor releases the mutex
     }
@@ -174,15 +197,13 @@ ACE_Future<T>::ACE_Future (const T &r)
   : future_rep_ (Future_Rep::create ())
 {
   ACE_DEBUG ((LM_DEBUG," (%t) funny constructor\n"));
-  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, ace_mon, this->mutex_));
   this->future_rep_->set (r);
 }
 
 template <class T> 
 ACE_Future<T>::~ACE_Future (void)
 {
-  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, ace_mon, this->mutex_));
-  Future_Rep::detach (future_rep_);
+  FUTURE_REP::detach (future_rep_);
 }
 
 template <class T> int 
@@ -204,17 +225,13 @@ ACE_Future<T>::cancel (const T &r)
   return this->future_rep_->set (r);
 }
 
-template <class T> void
+template <class T> int
 ACE_Future<T>::cancel (void)
-{
-  ACE_MT (ACE_GUARD_RETURN (ACE_Thread_Mutex, ace_mon, this->mutex_, -1));
-  
+{  
   // If this ACE_Future is already attached to a ACE_Future_Rep,
   // detach it (maybe delete the ACE_Future_Rep).
-  Future_Rep *new_rep (Future_Rep::create ());
-  Future_Rep *old_rep (this->future_rep_);
-  this->future_rep_ = new_rep;
-  Future_Rep::detach (old_rep);
+  FUTURE_REP::assign (this->future_rep_, FUTURE_REP::create ());
+  return 0;
 }
 
 template <class T> int
@@ -233,8 +250,6 @@ ACE_Future<T>::ready (void)
 template <class T> int
 ACE_Future<T>::get (T &value, ACE_Time_Value *tv)
 {
-  ACE_MT (ACE_GUARD_RETURN (ACE_Thread_Mutex, ace_mon, this->mutex_, -1));
-
   // We return the ACE_Future_rep.
   return this->future_rep_->get (value, tv);
 }
@@ -258,27 +273,16 @@ ACE_Future<T>::operator T ()
 }
 
 template <class T> void
-ACE_Future<T>::operator = (const ACE_Future<T> &r) 
+ACE_Future<T>::operator = (const ACE_Future<T> &rhs) 
 {
   // assignment:
   //
   //  bind <this> to the same <ACE_Future_Rep> as <r>.
 
-  Future_Rep *new_rep = 0;
-  {
-    // This will work if &r == this
-    // and if not it will keep the r.mutex_ locked for as little 
-    // time as possible
-    ACE_MT (ACE_GUARD (ACE_Thread_Mutex, ace_mon, r.mutex_));
-    new_rep = Future_Rep::attach (r.future_rep_);
-  }
-  
-  ACE_MT (ACE_GUARD (ACE_Thread_Mutex, ace_mon, this->mutex_));
-
-  Future_Rep *old_rep = this->future_rep_;
-  this->future_rep_ = new_rep;
-  // Setup the new rep before detaching old rep for exception safety.
-  Future_Rep::detach (old_rep);
+  // This will work if &r == this, by first increasing the ref count
+  ACE_Future<T> &r = ( ACE_Future<T> &) rhs;
+  FUTURE_REP::assign (this->future_rep_, 
+		      FUTURE_REP::attach (r.future_rep_));
 }
 
 template <class T> void 
@@ -288,8 +292,6 @@ ACE_Future<T>::dump (void) const
 
   if (this->future_rep_)
     this->future_rep_->dump ();
-
-  this->mutex_.dump ();
   ACE_DEBUG ((LM_DEBUG, ACE_END_DUMP));
 }
 
diff --git a/ace/Future.h b/ace/Future.h
index ab371b97323..09d08f5fdcd 100644
--- a/ace/Future.h
+++ b/ace/Future.h
@@ -39,14 +39,36 @@ template <class T> class ACE_Future_Rep
   friend class ACE_Future<T>;
 
 private:
+  
+  // Create, attach, detach and assign encapsulates the reference 
+  // count handling and the object lifetime of ACE_Future_Rep<T>
+  // instances.
+  
   static ACE_Future_Rep<T> *create (void);
-  static ACE_Future_Rep<T> *attach (ACE_Future_Rep<T> *rep);
-  static void detach (ACE_Future_Rep<T> *rep);
+  // Create a ACE_Future_Rep<T> and initialize the reference count
+  
+  static ACE_Future_Rep<T> *attach (ACE_Future_Rep<T> *&rep);
+  // Precondition(rep != 0)
+  // Increase the reference count and return argument. Uses
+  // the attribute "value_ready_mutex_" to synchronize reference
+  // count updating
+  
+  static void detach (ACE_Future_Rep<T> *&rep);
+  // Precondition(rep != 0)
+  // Decreases the reference count and and deletes rep if
+  // there are no more references to rep.
+  
+  static void assign (ACE_Future_Rep<T> *&rep,
+		      ACE_Future_Rep<T> *new_rep);
+  // Precondition(rep != 0 && new_rep != 0)
+  // Decreases the rep's reference count and and deletes rep if there
+  // are no more references to rep. Then assigns new_rep to rep
   
   int set (const T &r);
   // Set the result value.
 
-  int get (T &value, ACE_Time_Value *tv);
+  int get (T &value,
+	   ACE_Time_Value *tv);
   // Wait up to <tv> time to get the <value>.
 
   operator T ();
@@ -64,7 +86,7 @@ private:
   ACE_ALLOC_HOOK_DECLARE;
   // Declare the dynamic allocation hooks.
 
-  // = constructor and destructor private
+  // = Constructor and destructor private
   ACE_Future_Rep (void);
   ~ACE_Future_Rep (void);
 
@@ -75,7 +97,6 @@ private:
   // Pointer to the result.
 
   int ref_count_;
-//  ACE_Atomic_Op<ACE_Thread_Mutex, int> ref_count_;
   // Reference count.
 
   // = Condition variable and mutex that protect the <value_>.
@@ -90,6 +111,7 @@ template <class T> class ACE_Future
   //     method invocations.
   //
   // = DESCRIPTION
+  //    @@ Please update me...
 {
 public:
   // = Initialization and termination methods.
@@ -115,9 +137,11 @@ public:
   // Cancel an <ACE_Future> and assign the value <r>.  It is used if a
   // client does not want to wait for <T> to be produced.
   
-  void cancel (void);
-  // Cancel an <ACE_Future>.  This puts the future into its initial
-  // state.
+  int cancel (void);
+  // Cancel an <ACE_Future>.  Put the future into its initial
+  // state. Returns 0 on succes and -1 on failure. It is now possible
+  // to reuse the ACE_Future<T>. But remember, the ACE_Future<T>
+  // is now bound to a new ACE_Future_Rep<T>.
 
   int operator == (const ACE_Future<T> &r) const;
   // Equality operator that returns 1 if both ACE_Future<T> objects
@@ -132,7 +156,8 @@ public:
   // Make the result available. Is used by the server thread to give
   // the result to all waiting clients.
 
-  int get (T &value, ACE_Time_Value *tv = 0);
+  int get (T &value,
+	   ACE_Time_Value *tv = 0);
   // Wait up to <tv> time to get the <value>.
 
   operator T ();
@@ -164,10 +189,8 @@ private:
   // Do not allow address-of operator.
 
   // the ACE_Future_Rep
-  typedef ACE_Future_Rep<T> Future_Rep;
-  Future_Rep* future_rep_;
-
-  ACE_Thread_Mutex mutex_;
+  typedef ACE_Future_Rep<T> FUTURE_REP;
+  FUTURE_REP *future_rep_;
   // Protect operations on the <Future>.
 };
 
@@ -181,3 +204,4 @@ private:
 
 #endif /* ACE_HAS_THREADS */
 #endif /* ACE_FUTURE_H */
+
diff --git a/tests/Future_Test.cpp b/tests/Future_Test.cpp
index 6c42e795d08..338b063cb4d 100644
--- a/tests/Future_Test.cpp
+++ b/tests/Future_Test.cpp
@@ -12,8 +12,8 @@
 //    This example tests the ACE Future.
 //
 // = AUTHOR
-//    Andres Kruse <Andres.Kruse@cern.ch> and Douglas C. Schmidt
-//    <schmidt@cs.wustl.edu> 
+//    Andres Kruse <Andres.Kruse@cern.ch>, Douglas C. Schmidt
+//    <schmidt@cs.wustl.edu> and Per Andersson <pera@ipso.se>
 // 
 // ============================================================================
 
@@ -92,10 +92,10 @@ Method_Object_work::Method_Object_work (Scheduler* new_Scheduler,
 				        u_long new_param, 
 				        int new_count, 
 					ACE_Future<u_long> &new_result)
-  :   scheduler_ (new_Scheduler),
-      param_ (new_param),
-      count_ (new_count),
-      future_result_ (new_result)
+  : scheduler_ (new_Scheduler),
+    param_ (new_param),
+    count_ (new_count),
+    future_result_ (new_result)
 { 
   ACE_DEBUG ((LM_DEBUG,
 	      "(%t) Method_Object_work created\n"));
@@ -103,14 +103,17 @@ Method_Object_work::Method_Object_work (Scheduler* new_Scheduler,
 
 Method_Object_work::~Method_Object_work (void)
 {
-  ACE_DEBUG ((LM_DEBUG, "(%t) Method_Object_work will be deleted.\n"));
+  ACE_DEBUG ((LM_DEBUG, 
+	      "(%t) Method_Object_work will be deleted.\n"));
 }
 
 
 int
 Method_Object_work::call (void)
 {
-  return this->future_result_.set (this->scheduler_->work_i (this->param_, this->count_));
+  return this->future_result_.set 
+    (this->scheduler_->work_i (this->param_, 
+			       this->count_));
 }
 
 class Method_Object_name : public ACE_Method_Object
@@ -118,7 +121,8 @@ class Method_Object_name : public ACE_Method_Object
   //     Reification of the <name> method.
 {
 public:
-  Method_Object_name (Scheduler *, ACE_Future<const char*> &);
+  Method_Object_name (Scheduler *, 
+		      ACE_Future<const char*> &);
   virtual ~Method_Object_name (void);
   virtual int call (void);
 
@@ -153,27 +157,36 @@ class Method_Object_end : public ACE_Method_Object
   //     Reification of the <end> method.
 {
 public:
-  Method_Object_end (Scheduler *new_Scheduler): scheduler_ (new_Scheduler) {}
+  Method_Object_end (Scheduler *new_Scheduler)
+    : scheduler_ (new_Scheduler) {}
   virtual ~Method_Object_end (void) {}
-  virtual int call (void) { this->scheduler_->close (); return -1; }
+  virtual int call (void) { 
+    this->scheduler_->close (); 
+    return -1; 
+  }
 
 private:
   Scheduler *scheduler_;
 };
 
 // constructor
-Scheduler::Scheduler (const char *newname, Scheduler *new_Scheduler)
+Scheduler::Scheduler (const char *newname, 
+		      Scheduler *new_Scheduler)
 {
   ACE_NEW (this->name_, char[ACE_OS::strlen (newname) + 1]);
   ACE_OS::strcpy ((char *) this->name_, newname);
   this->scheduler_ = new_Scheduler;
-  ACE_DEBUG ((LM_DEBUG, "(%t) Scheduler %s created\n", this->name_));
+  ACE_DEBUG ((LM_DEBUG, 
+	      "(%t) Scheduler %s created\n", 
+	      this->name_));
 }
 
 // Destructor
 Scheduler::~Scheduler (void)
 {
-  ACE_DEBUG ((LM_DEBUG, "(%t) Scheduler %s will be destroyed\n", this->name_));
+  ACE_DEBUG ((LM_DEBUG, 
+	      "(%t) Scheduler %s will be destroyed\n", 
+	      this->name_));
   delete[] this->name_;
 }
 
@@ -182,7 +195,10 @@ int
 Scheduler::open (void *)
 {
   task_count++;
-  ACE_DEBUG ((LM_DEBUG, "(%t) Scheduler %s open\n", this->name_));
+  ACE_DEBUG ((LM_DEBUG, 
+	      "(%t) Scheduler %s open\n", 
+	      this->name_));
+  // Become an Active Object.
   return this->activate (THR_BOUND);
 }
 
@@ -190,7 +206,9 @@ Scheduler::open (void *)
 int 
 Scheduler::close (u_long)
 {
-  ACE_DEBUG ((LM_DEBUG, "(%t) Scheduler %s close\n", this->name_));
+  ACE_DEBUG ((LM_DEBUG, 
+	      "(%t) Scheduler %s close\n", 
+	      this->name_));
   task_count--;
   return 0;
 }
@@ -269,7 +287,8 @@ Scheduler::work (u_long newparam, int newcount)
     ACE_Future<u_long> new_future;
 
     this->activation_queue_.enqueue
-      (new Method_Object_work (this, newparam, newcount, new_future));
+      (new Method_Object_work (this, newparam, 
+			       newcount, new_future));
     return new_future;
   }
 }
@@ -317,7 +336,8 @@ main (int, char *[])
 	ACE_Future<u_long> fresulta, fresultb, fresultc, fresultd, fresulte;
 	ACE_Future<const char*> fname;
 
-	ACE_DEBUG ((LM_DEBUG, "(%t) going to do a non-blocking call\n"));
+	ACE_DEBUG ((LM_DEBUG, 
+		    "(%t) going to do a non-blocking call\n"));
 
 	fresulta = andres->work (9013);
 	fresultb = peter->work (9013);
@@ -327,9 +347,11 @@ main (int, char *[])
 
 	// see if the result is available...
 	if (fresulta.ready ())
-	  ACE_DEBUG ((LM_DEBUG, "(%t) wow.. work is ready.....\n"));
+	  ACE_DEBUG ((LM_DEBUG,
+		      "(%t) wow.. work is ready.....\n"));
 
-	ACE_DEBUG ((LM_DEBUG, "(%t) non-blocking call done... now blocking...\n"));
+	ACE_DEBUG ((LM_DEBUG,
+		    "(%t) non-blocking call done... now blocking...\n"));
 
 	// Save the result of fresulta.
 
@@ -390,7 +412,86 @@ main (int, char *[])
 	      (int) capsule_count,
 	      (int) methodobject_count));
 
-  ACE_DEBUG ((LM_DEBUG,"(%t) th' that's all folks!\n"));
+  {
+    // Check if set then get works, older versions of ACE_Future
+    // will lock forever (or until the timer expires), will use a small
+    // timer value to avoid blocking the process.
+   
+    ACE_Future<int> f1;
+    f1.set(100);
+   
+    ACE_Time_Value timeout(1);
+    int value = 0;
+
+    if (f1.get (value, &timeout) == 0 && value == 100)
+      ACE_DEBUG ((LM_DEBUG,
+		  "Ace_Future<T>::Set followed by Ace_Future<T>::Get works.\n"));
+    else
+      ACE_DEBUG ((LM_DEBUG,
+		  "ACE_Future<T>::Set followed by Ace_Future<T>::Get does "
+		  "not work, broken Ace_Future<> implementation.\n"));
+  }
+  {
+    ACE_DEBUG ((LM_DEBUG,
+		"Checking if Ace_Future<T>::operator= is implemented "
+		"incorrectly this might crash the program.\n"));
+
+    ACE_Future<int> f1;
+    {
+      ACE_Future<int> f2 (f1); // To ensure that a rep object is created
+    }
+    // Now it is one ACE_Future<int> referencing the rep instance
+      
+    ACE_DEBUG ((LM_DEBUG, "0.\n"));
+    //check that self assignment works
+    f1 = f1; 
+    // Is there any repesentation left, and if so what is the ref
+    // count older ACE_Future<> implementations have deleted the rep
+    // instance at this moment
+   
+    // The stuff below might crash the process if the op=
+    // implementation was bad
+    int value = 0;
+    ACE_Time_Value timeout (1);
+
+    f1.set (100);      
+    f1.get (value, &timeout);
+
+    ACE_DEBUG ((LM_DEBUG, "1.\n"));
+    { // Might delete the same data a couple of times  
+      ACE_Future<int> f2 (f1);
+      f1.set (100);      
+      f1.get (value, &timeout);
+    }
+
+    ACE_DEBUG ((LM_DEBUG, "2.\n"));
+    {   
+      ACE_Future<int> f2 (f1);
+      f1.set (100);      
+      f1.get (value, &timeout);
+    }
+
+    ACE_DEBUG ((LM_DEBUG, "3.\n"));
+    {  
+      ACE_Future<int> f2 (f1);
+      f1.set (100);      
+      f1.get (value, &timeout);
+    }
+    ACE_DEBUG ((LM_DEBUG, "4.\n"));
+    {  
+      ACE_Future<int> f2 (f1); 
+      f1.set (100);      
+      f1.get (value, &timeout);
+    }
+    ACE_DEBUG ((LM_DEBUG, "5.\n"));
+    {  
+      ACE_Future<int> f2 (90);
+      f2.get (value, &timeout);
+      f1.get (value, &timeout);
+    }
+  }
+  ACE_DEBUG ((LM_DEBUG, 
+	      "No it did not crash the program.\n"));
 
   ACE_OS::sleep (5);