[Aptitude-svn-commit] r4099 - in branches/aptitude-0.3/aptitude: . src/generic

Daniel Burrows dburrows at costa.debian.org
Thu Sep 15 23:17:17 UTC 2005


Author: dburrows
Date: Thu Sep 15 23:17:14 2005
New Revision: 4099

Modified:
   branches/aptitude-0.3/aptitude/ChangeLog
   branches/aptitude-0.3/aptitude/src/generic/resolver_manager.cc
   branches/aptitude-0.3/aptitude/src/generic/resolver_manager.h
Log:
Cleaner, better threading support.

Modified: branches/aptitude-0.3/aptitude/ChangeLog
==============================================================================
--- branches/aptitude-0.3/aptitude/ChangeLog	(original)
+++ branches/aptitude-0.3/aptitude/ChangeLog	Thu Sep 15 23:17:14 2005
@@ -1,5 +1,15 @@
 2005-09-15  Daniel Burrows  <dburrows at debian.org>
 
+	* src/generic/resolver_manager.cc, src/generic/resolver_manager.h:
+
+	  Rewrite the threading support to allow multiple simultaneous
+	  jobs in the background thread; the foreground thread now just
+	  posts jobs to the background thread and waits for a reply.
+	  Furthermore, the complicated dance of constantly
+	  stopping/starting the background thread has been removed in
+	  favor of a scheme where the thread is started once and then
+	  suspended using condition variables.
+
 	* src/generic/problemresolver/problemresolver.h:
 
 	  Allow the cancellation of a find_next_solution call to be

Modified: branches/aptitude-0.3/aptitude/src/generic/resolver_manager.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/generic/resolver_manager.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/generic/resolver_manager.cc	Thu Sep 15 23:17:14 2005
@@ -28,20 +28,22 @@
 
 #include <sigc++/functors/mem_fun.h>
 
-std::string ResolverManagerThreadClashException::errmsg() const
-{
-  return "Internal error: attempt to run two simultaneous resolvers";
-}
-
 // NB: we need a recursive mutex because some routines can be called
 // either by other routines of the class (already have a mutex lock)
 // or by the user (don't have a mutex lock); I could sidestep this
 // with some clever magic, but there's no point unless it turns out to
 // be a bottleneck.
 resolver_manager::resolver_manager(aptitudeDepCache *_cache)
-  :cache(_cache), resolver(0), selected_solution(0), out_of_time(false),
-   out_of_solutions(false), background_resolver_active(false),
-   resolver_suspend_count(0), continuation(NULL), resolver_thread(NULL),
+  :cache(_cache),
+   resolver(NULL),
+   selected_solution(0),
+   out_of_time(false),
+   out_of_solutions(false),
+   background_thread_killed(false),
+   resolver_null(true),
+   background_thread_suspend_count(0),
+   background_thread_in_resolver(false),
+   resolver_thread(NULL),
    mutex(threads::mutex::attr(PTHREAD_MUTEX_RECURSIVE_NP))
 {
   cache->pre_package_state_changed.connect(sigc::mem_fun(this, &resolver_manager::discard_resolver));
@@ -50,13 +52,17 @@
   aptcfg->connect(PACKAGE "::Recommends-Important",
 		  sigc::mem_fun(this,
 				&resolver_manager::discard_resolver));
+
+  start_background_thread();
+
+  maybe_create_resolver();
 }
 
 resolver_manager::~resolver_manager()
 {
   discard_resolver();
 
-  delete continuation;
+  kill_background_thread();
 
   for(unsigned int i = 0; i < solutions.size(); ++i)
     delete solutions[i];
@@ -69,117 +75,158 @@
 class resolver_manager::background_suspender
 {
   resolver_manager &m;
-  /** A lock on the manager's main mutex; used as a way of reminding
-   *  myself to take that lock first.
-   */
-  threads::mutex::lock &l;
 public:
-  background_suspender(resolver_manager &_m,
-		       threads::mutex::lock &_l)
-    :m(_m), l(_l)
-  {
-    if(m.resolver_suspend_count == 0)
-      m.stop_background_resolver();
-    ++m.resolver_suspend_count;
+  background_suspender(resolver_manager &_m)
+    :m(_m)
+  {
+    m.suspend_background_thread();
   }
 
   ~background_suspender()
   {
-    --m.resolver_suspend_count;
-    if(m.resolver_suspend_count == 0)
-      m.restart_background_resolver();
+    m.unsuspend_background_thread();
   }
 };
 
 // This assumes that background_resolver_active is empty when it
 // starts (see restart_background_resolver)
 //
-// TODO: merge with the foreground stuff?
-void resolver_manager::background_thread_execution(int max_steps,
-						   int sol_num)
+// FIXME: max_steps should be changed when the configuration is (not a
+// visible bug at the moment since you can't change that option
+// interactively)
+void resolver_manager::background_thread_execution(int max_steps)
 {
-  background_resolver_active.put(true);
+  threads::mutex::lock l(background_control_mutex);
 
-  try
+  while(1)
     {
-      aptitude_resolver::solution *sol = do_get_solution(sol_num);
-
-      continuation->success(*sol);
+      while(background_thread_suspend_count > 0 || resolver_null ||
+	    (!background_thread_killed && pending_jobs.empty()))
+	background_control_cond.wait(l);
+
+      if(background_thread_killed)
+	break;
+
+      job_request job = pending_jobs.top();
+      pending_jobs.pop();
+      background_thread_in_resolver = true;
+      background_resolver_cond.wake_all();
+      l.release();
 
-      assert(background_resolver_active.take());
-      background_resolver_active.put(false);
-    }
-  // If the computation was interrupted, just terminate silently
-  // without clearing the background_resolver_active field.
-  catch(InterruptedException)
-    {
-      assert(background_resolver_active.take());
-      background_resolver_active.put(true);
-    }
-  catch(NoMoreSolutions)
-    {
-      continuation->no_more_solutions();
+      try
+	{
+	  aptitude_resolver::solution *sol = do_get_solution(max_steps,
+							     job.sol_num);
+	  job.k->success(*sol);
+	}
+      catch(InterruptedException)
+	{
+	}
+      catch(NoMoreSolutions)
+	{
+	  job.k->no_more_solutions();
+	}
+      catch(NoMoreTime)
+	{
+	  job.k->no_more_time();
+	}
 
-      assert(background_resolver_active.take());
-      background_resolver_active.put(false);
-    }
-  catch(NoMoreTime)
-    {
-      continuation->no_more_time();
+      l.acquire();
+      delete job.k;
 
-      assert(background_resolver_active.take());
-      background_resolver_active.put(false);
+      background_thread_in_resolver = false;
+      background_resolver_cond.wake_all();
     }
-  // Other escaping exceptions should blow up the program, so don't
-  // worry about the state of background_resolver_active here.
 }
 
 // Need this because sigc slots aren't threadsafe :-(
 struct resolver_manager::background_thread_bootstrap
 {
   int max_steps;
-  int sol_num;
   resolver_manager &m;
 public:
   background_thread_bootstrap(int _max_steps,
-			      int _sol_num,
 			      resolver_manager &_m)
-    :max_steps(_max_steps), sol_num(_sol_num), m(_m)
+    :max_steps(_max_steps), m(_m)
   {
   }
 
   void operator()()
   {
-    m.background_thread_execution(max_steps, sol_num);
+    m.background_thread_execution(max_steps);
   }
 };
 
-void resolver_manager::restart_background_resolver()
+void resolver_manager::start_background_thread()
 {
   threads::mutex::lock l(mutex);
 
-  if(background_resolver_active.take())
-    {
-      assert(resolver_thread == NULL);
-      resolver_thread = new threads::thread(background_thread_bootstrap(aptcfg->FindI(PACKAGE "::ProblemResolver::StepLimit", 5000), background_target, *this));
-    }
-  else
-    background_resolver_active.put(false);
+  if(resolver_thread == NULL)
+    resolver_thread = new threads::thread(background_thread_bootstrap(aptcfg->FindI(PACKAGE "::ProblemResolver::StepLimit", 5000), *this));
 }
 
-void resolver_manager::stop_background_resolver()
+void resolver_manager::kill_background_thread()
 {
   threads::mutex::lock l(mutex);
 
-  if(background_resolver_active.take())
+  if(resolver_thread != NULL)
     {
-      background_resolver_active.put(true);
-      assert(resolver_thread != NULL);
+      threads::mutex::lock control_lock(background_control_mutex);
+
       resolver->cancel_solver();
+      background_thread_killed = true;
+      background_control_cond.wake_all();
+
+      control_lock.release();
+
       resolver_thread->join();
+      delete resolver_thread;
+      resolver_thread = NULL;
+
+
+      // Reset the associated data structures.
+      control_lock.acquire();
+      pending_jobs = std::priority_queue<job_request, std::vector<job_request>, job_request_sol_num_compare>();
+      background_thread_killed = false;
+      background_thread_suspend_count = 0;
+      background_thread_in_resolver = false;
     }
-  else
-    background_resolver_active.put(false);
+}
+
+void resolver_manager::suspend_background_thread()
+{
+  threads::mutex::lock l(mutex);
+
+  // May occur due to background_suspend objects existing while
+  // kill_background_thread runs.
+  if(resolver_thread == NULL)
+    return;
+
+  threads::mutex::lock control_lock(background_control_mutex);
+
+  resolver->cancel_solver();
+
+  ++background_thread_suspend_count;
+  background_control_cond.wake_all();
+
+  while(background_thread_in_resolver)
+    background_resolver_cond.wait(control_lock);
+
+  resolver->uncancel_solver();
+}
+
+void resolver_manager::unsuspend_background_thread()
+{
+  threads::mutex::lock l(mutex);
+
+  if(resolver_thread == NULL)
+    return;
+
+  threads::mutex::lock control_lock(background_control_mutex);
+
+  assert(background_thread_suspend_count > 0);
+  --background_thread_suspend_count;
+  background_control_cond.wake_all();
 }
 
 void resolver_manager::maybe_create_resolver()
@@ -194,13 +241,10 @@
 {
   threads::mutex::lock l(mutex);
 
-  stop_background_resolver();
-  // Now we know no background resolver is running, and we've locked
-  // out the other methods of this object, so this is guaranteed to be
-  // safe.
-  bool b;
-  assert(background_resolver_active.try_take(b));
-  background_resolver_active.put(false);
+  if(resolver == NULL)
+    return;
+
+  background_suspender bs(*this);
 
   delete resolver;
 
@@ -210,13 +254,23 @@
     selected_solution = 0;
   }
 
-  resolver=NULL;
+  resolver = NULL;
 
-  out_of_time.take();
-  out_of_time.put(false);
+  {
+    threads::mutex::lock l2(background_control_mutex);
+    resolver_null = true;
+    while(!pending_jobs.empty())
+      {
+	delete pending_jobs.top().k;
+	pending_jobs.pop();
+      }
+    background_control_cond.wake_all();
+  }
 
+  out_of_time.take();
   out_of_solutions.take();
   out_of_solutions.put(false);
+  out_of_time.put(false);
 }
 
 void resolver_manager::create_resolver()
@@ -261,12 +315,18 @@
 				aptcfg->FindI(PACKAGE "::ProblemResolver::StandardScore", 3),
 				aptcfg->FindI(PACKAGE "::ProblemResolver::OptionalScore", 1),
 				aptcfg->FindI(PACKAGE "::ProblemResolver::ExtraScore", -1));
+
+  {
+    threads::mutex::lock l2(background_control_mutex);
+    resolver_null = false;
+    background_control_cond.wake_all();
+  }
 }
 
 void resolver_manager::set_debug(bool activate)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver_exists());
 
@@ -288,16 +348,17 @@
   return solutions.size();
 }
 
-bool resolver_manager::solution_generation_complete() const
+bool resolver_manager::solution_generation_complete() // const
 {
   threads::mutex::lock l(mutex);
 
-  // FIXME: does this need a lock in the resolver?
-  // background_suspender bs(*this, l);
-  return resolver->exhausted();
+  bool rval = out_of_solutions.take();
+  out_of_solutions.put(rval);
+
+  return rval;
 }
 
-bool resolver_manager::solutions_exhausted() const
+bool resolver_manager::solutions_exhausted() // const
 {
   threads::mutex::lock l(mutex);
 
@@ -318,7 +379,7 @@
     return selected_solution == 0;
 }
 
-aptitude_resolver::solution *resolver_manager::do_get_solution(unsigned int solution_num)
+aptitude_resolver::solution *resolver_manager::do_get_solution(unsigned int max_steps, unsigned int solution_num)
 {
   threads::mutex::lock sol_l(solutions_mutex);
   if(solution_num < solutions.size())
@@ -330,27 +391,31 @@
 
       try
 	{
-	  generic_solution<aptitude_universe> sol = resolver->find_next_solution(aptcfg->FindI(PACKAGE "::ProblemResolver::StepLimit", 5000));
+	  generic_solution<aptitude_universe> sol = resolver->find_next_solution(max_steps);
 
 	  sol_l.acquire();
 	  solutions.push_back(new aptitude_resolver::solution(sol.clone()));
 	  sol_l.release();
 
 	  out_of_time.take();
+	  out_of_solutions.take();
+	  out_of_solutions.put(false);
 	  out_of_time.put(false);
 	}
       catch(NoMoreTime)
 	{
 	  out_of_time.take();
+	  out_of_solutions.take();
+	  out_of_solutions.put(false);
 	  out_of_time.put(true);
 	  throw NoMoreTime();
 	}
       catch(NoMoreSolutions)
 	{
 	  out_of_time.take();
-	  out_of_time.put(false);
 	  out_of_solutions.take();
 	  out_of_solutions.put(true);
+	  out_of_time.put(false);
 	  throw NoMoreSolutions();
 	}
     }
@@ -358,13 +423,83 @@
   return solutions[solution_num];
 }
 
+/** A continuation that works by either placing \b true in the Boolean
+ *  variable corresponding to the thrown exception, or updating the
+ *  given solution, then signalling the given condition.  Note that
+ *  this only works because we expect to be able to
+ */
+class solution_return_continuation : public resolver_manager::background_continuation
+{
+  const generic_solution<aptitude_universe> * &sol;
+  bool &oot;
+  bool &oos;
+  threads::mutex &m;
+  threads::condition &c;
+public:
+  solution_return_continuation(const generic_solution<aptitude_universe> * &_sol,
+			       bool &_oot,
+			       bool &_oos,
+			       threads::mutex &_m,
+			       threads::condition &_c)
+    :sol(_sol), oot(_oot), oos(_oos), m(_m), c(_c)
+  {
+  }
+
+  void success(const generic_solution<aptitude_universe> &result)
+  {
+    threads::mutex::lock l(m);
+
+    sol = &result;
+    c.wake_all();
+  }
+
+  void no_more_time()
+  {
+    threads::mutex::lock l(m);
+
+    oot = true;
+    c.wake_all();
+  }
+
+  void no_more_solutions()
+  {
+    threads::mutex::lock l(m);
+    oos = true;
+    c.wake_all();
+  }
+
+  void interrupted()
+  {
+    // Should never happen, since we hold the big lock.
+    abort();
+  }
+};
+
 const aptitude_resolver::solution &resolver_manager::get_solution(unsigned int solution_num)
 {
   threads::mutex::lock l(mutex);
 
-  background_suspender bs(*this, l);
+  assert(resolver);
+
+
+  const generic_solution<aptitude_universe> *sol = NULL;
+  bool oot = false;
+  bool oos = false;
+  threads::mutex m;
+  threads::condition c;
+
+  get_solution_background(solution_num, new solution_return_continuation(sol, oot, oos, m, c));
+  threads::mutex::lock cond_l(m);
+
+  while(!sol && !oot && !oos)
+    c.wait(cond_l);
 
-  return *do_get_solution(solution_num);
+  if(oot)
+    throw NoMoreTime();
+  else if(oos)
+    throw NoMoreSolutions();
+
+  return *sol;
 }
 
 
@@ -374,34 +509,24 @@
 
   assert(resolver);
 
-  bool oot = out_of_time.take();
-  out_of_time.put(oot);
+  const generic_solution<aptitude_universe> *sol = NULL;
+  bool oot = false;
+  bool oos = false;
+  threads::mutex m;
+  threads::condition c;
 
-  // Only throw NoMoreTime when we're trying to generate a new
-  // solution.
-  threads::mutex::lock sol_l(solutions_mutex);
-  oot = oot && selected_solution >= solutions.size();
-  sol_l.release();
+  get_current_solution_background(new solution_return_continuation(sol, oot, oos, m, c));
+  threads::mutex::lock cond_l(m);
+
+  while(!sol && !oot && !oos)
+    c.wait(cond_l);
 
   if(oot)
     throw NoMoreTime();
-  else
-    {
-      sol_l.acquire();
-      unsigned int start_size = solutions.size();
-      sol_l.release();
-
-      const aptitude_resolver::solution &rval = get_solution(selected_solution);
-
-      sol_l.acquire();
-      if(start_size != solutions.size())
-	{
-	  sol_l.release();
-	  selected_solution_changed();
-	}
+  else if(oos)
+    throw NoMoreSolutions();
 
-      return rval;
-    }
+  return *sol;
 }
 
 void resolver_manager::get_solution_background(unsigned int solution_num,
@@ -409,30 +534,23 @@
 {
   threads::mutex::lock l(mutex);
 
+  assert(resolver_exists());
+
   threads::mutex::lock sol_l(solutions_mutex);
   if(solution_num < solutions.size())
     {
-      k->success(*solutions[solution_num]);
+      generic_solution<aptitude_universe> *sol = solutions[solution_num];
+      sol_l.release();
+
+      k->success(*sol);
       return;
     }
   sol_l.release();
 
 
-  bool active = background_resolver_active.take();
-  background_resolver_active.put(active);
-
-  if(active)
-    throw ResolverManagerThreadClashException();
-  else
-    {
-      // Prime the background thread:
-      background_resolver_active.take();
-      background_resolver_active.put(true);
-
-      background_target = solution_num;
-      continuation = k;
-      restart_background_resolver();
-    }
+  threads::mutex::lock control_lock(background_control_mutex);
+  pending_jobs.push(job_request(solution_num, k));
+  background_control_cond.wake_all();
 }
 
 void resolver_manager::get_current_solution_background(background_continuation *k)
@@ -441,17 +559,23 @@
 
   assert(resolver);
 
+
   bool oot = out_of_time.take();
+  bool oos = out_of_solutions.take();
+
   out_of_time.put(oot);
+  out_of_solutions.put(oos);
 
   // Only throw NoMoreTime when we're trying to generate a new
   // solution.
   threads::mutex::lock sol_l(solutions_mutex);
-  oot = oot && selected_solution >= solutions.size();
+  size_t solution_count = solutions.size();
   sol_l.release();
 
-  if(oot)
-    k->no_more_time();
+  if(oot && selected_solution >= solution_count)
+    throw NoMoreTime();
+  else if(oos && selected_solution >= solution_count)
+    throw NoMoreSolutions();
   else
     get_solution_background(selected_solution, k);
 }
@@ -459,23 +583,27 @@
 void resolver_manager::reject_version(const aptitude_resolver_version &ver)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
   resolver->reject_version(ver);
-  restart_background_resolver();
 }
 
 void resolver_manager::unreject_version(const aptitude_resolver_version &ver)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
   resolver->unreject_version(ver);
 
+  out_of_time.take();
+  out_of_solutions.take();
+  out_of_solutions.put(false);
+  out_of_time.put(false);
+
   selected_solution_changed();
 }
 
@@ -490,7 +618,7 @@
 void resolver_manager::mandate_version(const aptitude_resolver_version &ver)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
@@ -500,12 +628,17 @@
 void resolver_manager::unmandate_version(const aptitude_resolver_version &ver)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
   resolver->unmandate_version(ver);
 
+  out_of_time.take();
+  out_of_solutions.take();
+  out_of_solutions.put(false);
+  out_of_time.put(false);
+
   selected_solution_changed();
 }
 
@@ -520,7 +653,7 @@
 void resolver_manager::harden_dep(const aptitude_resolver_dep &dep)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
@@ -530,12 +663,17 @@
 void resolver_manager::unharden_dep(const aptitude_resolver_dep &dep)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
   resolver->unharden(dep);
 
+  out_of_time.take();
+  out_of_solutions.take();
+  out_of_solutions.put(false);
+  out_of_time.put(false);
+
   selected_solution_changed();
 }
 
@@ -550,7 +688,7 @@
 void resolver_manager::force_break_dep(const aptitude_resolver_dep &dep)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
@@ -560,13 +698,18 @@
 void resolver_manager::unforce_break_dep(const aptitude_resolver_dep &dep)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver);
 
   resolver->unforce_break(dep);
 
   selected_solution_changed();
+
+  out_of_time.take();
+  out_of_solutions.take();
+  out_of_solutions.put(false);
+  out_of_time.put(false);
 }
 
 bool resolver_manager::is_forced_broken(const aptitude_resolver_dep &dep)
@@ -621,7 +764,7 @@
 				   int score)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   assert(resolver_exists());
   assert(resolver->fresh());
@@ -633,7 +776,7 @@
 void resolver_manager::dump(ostream &out)
 {
   threads::mutex::lock l(mutex);
-  background_suspender bs(*this, l);
+  background_suspender bs(*this);
 
   if(!resolver_exists())
     return;

Modified: branches/aptitude-0.3/aptitude/src/generic/resolver_manager.h
==============================================================================
--- branches/aptitude-0.3/aptitude/src/generic/resolver_manager.h	(original)
+++ branches/aptitude-0.3/aptitude/src/generic/resolver_manager.h	Thu Sep 15 23:17:14 2005
@@ -35,6 +35,7 @@
 #include <sigc++/signal.h>
 #include <sigc++/trackable.h>
 
+#include <queue>
 #include <vector>
 
 class aptitudeDepCache;
@@ -44,15 +45,6 @@
 template<typename PackageUniverse> class generic_solution;
 class aptitude_resolver;
 
-/** An exception thrown when the user tries to start two threads at
- *  once.
- */
-class ResolverManagerThreadClashException : public Exception
-{
-public:
-  std::string errmsg() const;
-};
-
 /** Manages a resolver for a single cache object.  When broken
  *  packages arise, a new resolver is created; whenever the state of a
  *  package changes, the resolver is deleted and reset.  While a
@@ -97,6 +89,32 @@
   };
 
 private:
+  /** Information about a single request posted to the background
+   *  thread.
+   */
+  struct job_request
+  {
+    /** The solution number to be calculated. */
+    int sol_num;
+
+    /** The continuation of this computation. */
+    background_continuation *k;
+
+    job_request(int _sol_num, background_continuation *_k)
+      :sol_num(_sol_num), k(_k)
+    {
+    }
+  };
+
+  /** Sort job requests by their solution number. */
+  struct job_request_sol_num_compare
+  {
+    bool operator()(const job_request &jr1, const job_request &jr2) const
+    {
+      return jr1.sol_num < jr2.sol_num;
+    }
+  };
+
   /** The cache file on which this manager operates. */
   aptitudeDepCache *cache;
 
@@ -128,32 +146,53 @@
 
   /** If \b true, a find_next_solution() call will terminate with
    *  NoMoreSolutions.  (this may be the case even if it is \b false)
+   *
+   *  By convention, this box is accessed while out_of_time is empty
+   *  if both are to be accessed at once.
    */
   threads::box<bool> out_of_solutions;
 
-  /** If \b true, a background thread should be active to calculate a
-   *  resolver solution.  This is a box to eliminate a potential race
-   *  between the very end of the background thread and the 'start a
-   *  background thread' routine.
+  /** The pending job requests for the background thread.
+   */
+  std::priority_queue<job_request, std::vector<job_request>,
+		      job_request_sol_num_compare> pending_jobs;
+
+  /** If \b true, the background thread should abort its execution. */
+  bool background_thread_killed;
+
+  /** If \b true, the resolver is \b NULL.  (this is used rather than
+   *  checking the variable directly in order to make it painfully
+   *  clear what the proper locking protocol is)
    */
-  threads::box<bool> background_resolver_active;
+  bool resolver_null;
 
-  /** If background_resolver_active is \b true, then this is the
-   *  solution number which it is attempting to calculate.
+  /** The number of times the background thread has been suspended; it
+   *  will only be allowed to run if this value is 0.
    */
-  unsigned int background_target;
+  int background_thread_suspend_count;
 
-  /** Used to manage the background thread semi-automatically; counts
-   *  how many times it was suspended by the stack-based
-   *  background_suspend class.
+  /** If \b true, the background thread is currently running in the
+   *  resolver; this indicates that foreground threads trying to
+   *  suspend the background thread should wait on
+   *  background_in_resolver_cond until this becomes \b false.
    */
-  int resolver_suspend_count;
+  bool background_thread_in_resolver;
 
-  /** If background_resolver_active is \b true, this is a callback
-   *  object to be used in handling the result of the background
-   *  thread's computation.
+  /** A lock around pending_jobs, background_thread_killed,
+   *  background_thread_suspend_count, background_thread_in_resolver,
+   *  and resolver_null.
    */
-  background_continuation *continuation;
+  threads::mutex background_control_mutex;
+
+  /** A condition signalled for pending_jobs,
+   *  background_thread_killed, background_thread_suspend_count, and
+   *  resolver_null.
+   */
+  threads::condition background_control_cond;
+
+  /** A condition signalled for background_thread_in_resolver.
+   */
+  threads::condition background_resolver_cond;
 
   /** The thread in which a background resolver is running, or \b NULL
    *  if none is.
@@ -185,7 +224,7 @@
    *  the foreground or in the background.  It is called by
    *  background_thread_execution and get_solution.
    */
-  generic_solution<aptitude_universe> *do_get_solution(unsigned int solution_number);
+  generic_solution<aptitude_universe> *do_get_solution(unsigned int max_Steps, unsigned int solution_number);
 
   /** The actual background thread.
    *
@@ -193,18 +232,29 @@
    *  \param sol_num the number of solutions to be examined by this
    *                 thread.
    */
-  void background_thread_execution(int max_steps, int sol_num);
+  void background_thread_execution(int max_steps);
+
+  /** Start a background thread if none exists. */
+  void start_background_thread();
+
+  /** Destroy the background thread completely and reset its control
+   *  parameters.  Waits until the thread has terminated to return.
+   *
+   *  If no thread exists, do nothing.
+   */
+  void kill_background_thread();
 
-  /** The caller should hold the mutex; if no resolver thread is
-   *  currently running and background_resolver_active is \b true,
-   *  then a new resolver will be started.
+  /** Increments the suspend count of the background thread, and (if
+   *  necessary) interrupts a running resolution and waits for the
+   *  thread to leave the resolver.
    */
-  void restart_background_resolver();
+  void suspend_background_thread();
 
-  /** The caller should hold the mutex; stops any running resolver
-   *  thread, but leaves background_resolver_active set.
+  /** Decrements the suspend count of the background thread, and (if
+   *  necessary) unsuspends it.
    */
-  void stop_background_resolver();
+  void unsuspend_background_thread();
+
 
   /** Create a resolver if necessary. */
   void maybe_create_resolver();
@@ -300,13 +350,13 @@
   void get_current_solution_background(background_continuation *k);
 
   /** If \b true, all solutions have been generated. */
-  bool solution_generation_complete() const;
+  bool solution_generation_complete() /*const*/;
 
   /** If \b true, then either there is no resolver, or the resolver
    *  has been exhausted and the solution pointer is set to the last
    *  solution.
    */
-  bool solutions_exhausted() const;
+  bool solutions_exhausted() /* const */;
 
   /** If \b true, the solution pointer is set to the first
    *  solution.



More information about the Aptitude-svn-commit mailing list