[Aptitude-svn-commit] r4037 - in branches/aptitude-0.3/aptitude: . src src/vscreen

Daniel Burrows dburrows at costa.debian.org
Fri Sep 2 19:55:01 UTC 2005


Author: dburrows
Date: Fri Sep  2 19:54:57 2005
New Revision: 4037

Modified:
   branches/aptitude-0.3/aptitude/ChangeLog
   branches/aptitude-0.3/aptitude/configure.ac
   branches/aptitude-0.3/aptitude/src/Makefile.am
   branches/aptitude-0.3/aptitude/src/download.cc
   branches/aptitude-0.3/aptitude/src/download_bar.cc
   branches/aptitude-0.3/aptitude/src/download_list.cc
   branches/aptitude-0.3/aptitude/src/download_screen.cc
   branches/aptitude-0.3/aptitude/src/vscreen/Makefile.am
   branches/aptitude-0.3/aptitude/src/vscreen/curses++.cc
   branches/aptitude-0.3/aptitude/src/vscreen/vscreen.cc
   branches/aptitude-0.3/aptitude/src/vscreen/vscreen.h
Log:
Use threads for the core vscreen code.

Modified: branches/aptitude-0.3/aptitude/ChangeLog
==============================================================================
--- branches/aptitude-0.3/aptitude/ChangeLog	(original)
+++ branches/aptitude-0.3/aptitude/ChangeLog	Fri Sep  2 19:54:57 2005
@@ -1,5 +1,22 @@
 2005-09-02  Daniel Burrows  <dburrows at debian.org>
 
+	* configure.ac, src/download_bar.cc, src/download.cc, src/download_list.cc, src/download_screen.cc, src/vscreen/Makefile.am, src/vscreen/curses++.cc, src/vscreen/vscreen.cc, src/vscreen/vscreen.h:
+
+	  Explicitly support and use threading in the vscreen core.  The
+	  design is relatively conservative: only the "main" thread is
+	  permitted to access most vscreen stuff; other threads carry out
+	  vscreen tasks by "posting" events to the main loop; the events
+	  are run serially in the main loop.
+
+	  The core vscreen loop has been split into a small main loop and
+	  three auxillary threads that dispatch input, timeout, and
+	  asynchronous signal events to the main loop.  The resulting code
+	  should be a lot cleaner than what I had before, although there
+	  are still some warts that could be taken care of.
+	  Unfortunately, resizing seems to be slightly broken by this, or
+	  at least broken in a different way than it was before; the
+	  terminal sometimes displays incorrectly until you press a key.
+
 	* src/generic/threads.h:
 
 	  Support mutex attributes and add a recursive mutex type (for

Modified: branches/aptitude-0.3/aptitude/configure.ac
==============================================================================
--- branches/aptitude-0.3/aptitude/configure.ac	(original)
+++ branches/aptitude-0.3/aptitude/configure.ac	Fri Sep  2 19:54:57 2005
@@ -21,7 +21,7 @@
 AC_CHECK_LIB(apt-pkg, main, , [AC_MSG_ERROR([Can't find the APT libraries -- please install libapt-pkg-dev])])
 AC_CHECK_LIB(pthread, main,
 	HAVE_LIBPTHREAD=1
-	, [AC_MSG_WARN([Can't find the POSIX thread libraries -- configuring without threads])])
+	, [AC_MSG_ERROR([Can't find the POSIX thread libraries])])
 
 ALL_LINGUAS="ar bs ca cs da de el es eu fi fr gl it ja lt nb nl nn pl pt pt_BR ro ru sk tl tr vi zh_CN zh_TW"
 AM_GNU_GETTEXT

Modified: branches/aptitude-0.3/aptitude/src/Makefile.am
==============================================================================
--- branches/aptitude-0.3/aptitude/src/Makefile.am	(original)
+++ branches/aptitude-0.3/aptitude/src/Makefile.am	Fri Sep  2 19:54:57 2005
@@ -8,7 +8,7 @@
 
 bin_PROGRAMS=aptitude
 
-LDADD=@LIBINTL@ cmdline/libcmdline.a generic/libgeneric.a vscreen/config/libconf.a mine/libcmine.a vscreen/libvscreen.a
+LDADD=@LIBINTL@ cmdline/libcmdline.a vscreen/config/libconf.a mine/libcmine.a vscreen/libvscreen.a generic/libgeneric.a
 
 aptitude_SOURCES= 	\
 	aptitude.h	\

Modified: branches/aptitude-0.3/aptitude/src/download.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/download.cc	Fri Sep  2 19:54:57 2005
@@ -122,11 +122,6 @@
       return false;
     }
 
-  sigset_t signals,oldsigs;
-  sigemptyset(&signals);
-  sigaddset(&signals, SIGWINCH);
-  sigprocmask(SIG_UNBLOCK, &signals, &oldsigs);
-
   switch(fetcher.Run(aptcfg->FindI(PACKAGE "::UI::Download-Poll-Interval", 50000)))
     // At this point the widget should be deleted
     {
@@ -135,8 +130,6 @@
       for(pkgAcquireIterator i=fetcher.ItemsBegin(); i!=fetcher.ItemsEnd(); ++i)
 	(*i)->Finished();
 
-      sigprocmask(SIG_SETMASK, &oldsigs, &signals);
-
       download_progress->Complete();
 
       delete download_progress;
@@ -148,8 +141,6 @@
 	if((*i)->Status!=pkgAcquire::Item::StatDone)
 	  (*i)->Finished();
 
-      sigprocmask(SIG_SETMASK, &oldsigs, &signals);
-
       break;
     }
 
@@ -362,11 +353,6 @@
   // FIXME: console-apt checks for errors here.  I don't see anything that
   // could cause errors :)
 
-  sigset_t signals,oldsigs;
-  sigemptyset(&signals);
-  sigaddset(&signals, SIGWINCH);
-  sigprocmask(SIG_UNBLOCK, &signals, &oldsigs);
-
   // FIXME: the control logic in the following loop is very tortuous and
   //       should be simplified.
 
@@ -379,8 +365,6 @@
 
   while(!done)
     {
-      sigprocmask(SIG_SETMASK, &oldsigs, &signals);
-
       pkgAcquire::RunResult res=fetcher.Run(aptcfg->FindI(PACKAGE "::UI::Download-Poll-Interval", 50000));
       if(res!=pkgAcquire::Continue || abortable.get_aborted())
 	// We failed or were cancelled

Modified: branches/aptitude-0.3/aptitude/src/download_bar.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download_bar.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/download_bar.cc	Fri Sep  2 19:54:57 2005
@@ -37,18 +37,6 @@
 
 #include <sigc++/functors/ptr_fun.h>
 
-#define CRITICAL_ENTER	\
-  vscreen_acquirelock();\
-  sigset_t ___signals,___oldset;\
-  sigemptyset(&___signals);	\
-  sigaddset(&___signals, SIGWINCH);\
-  sigprocmask(SIG_BLOCK, &___signals, &___oldset);\
-  assert(!sigismember(&___oldset, SIGWINCH));
-
-#define CRITICAL_EXIT	\
-  sigprocmask(SIG_SETMASK, &___oldset, &___signals);\
-  assert(sigismember(&___signals, SIGWINCH));\
-  vscreen_releaselock();
 
 download_status_bar::download_status_bar()
 {
@@ -56,9 +44,7 @@
 
 bool download_status_bar::MediaChange(string media, string drive)
 {
-  CRITICAL_ENTER
-
-    fragment *f=wrapbox(fragf(_("Please insert the disc labeled \"%s\" into the drive \"%s\""),
+  fragment *f=wrapbox(fragf(_("Please insert the disc labeled \"%s\" into the drive \"%s\""),
 			      media.c_str(), drive.c_str()));
 
   vs_widget_ref w = vs_dialog_ok(f, arg(sigc::ptr_fun(vscreen_exitmain)),
@@ -66,8 +52,6 @@
   w->show_all();
   popup_widget(w);
 
-  CRITICAL_EXIT
-
   vscreen_mainloop();  // Eeeeeek!  Recursive mainloop!  I'm afraid..
 
   return true;
@@ -75,54 +59,38 @@
 
 void download_status_bar::IMSHit(pkgAcquire::ItemDesc &itm)
 {
-  CRITICAL_ENTER
-
   last_msg=_("Hit ")+itm.Description;
   last_switchtime=time(0);
 
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 void download_status_bar::Fetch(pkgAcquire::ItemDesc &itm)
 {
-  CRITICAL_ENTER
-
   last_msg=_("Downloading ")+itm.ShortDesc;
   last_switchtime=time(0);
 
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 void download_status_bar::Done(pkgAcquire::ItemDesc &itm)
 {
-  CRITICAL_ENTER
-
   last_msg=_("Got ")+itm.Description;
   last_switchtime=time(0);
 
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 void download_status_bar::Fail(pkgAcquire::ItemDesc &itm)
 {
-  CRITICAL_ENTER
-
   last_msg=itm.Description+": "+itm.Owner->ErrorText;
   last_switchtime=time(0);
 
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 bool download_status_bar::Pulse(pkgAcquire *Owner)
@@ -131,8 +99,6 @@
 
   if(difftime(time(0), last_switchtime)>1)
     {
-      CRITICAL_ENTER
-
       pkgAcquire::Worker *test=Owner->WorkersBegin();
       bool found=false;
       while(test)
@@ -179,8 +145,6 @@
 
       vscreen_update();
       vscreen_tryupdate();
-
-      CRITICAL_EXIT
     }
 
   return !cancelled;
@@ -232,13 +196,9 @@
 
 void download_status_bar::Stop()
 {
-  CRITICAL_ENTER
-
   pkgAcquireStatus::Stop();
 
   destroy();
-
-  CRITICAL_EXIT
 }
 
 bool download_status_bar::handle_key(const key &k)

Modified: branches/aptitude-0.3/aptitude/src/download_list.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download_list.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/download_list.cc	Fri Sep  2 19:54:57 2005
@@ -131,8 +131,6 @@
   vscreen_widget::destroy();
 }
 
-// all that CRITICAL_ENTER/CRITICAL_EXIT stuff is left out until I'm sure I
-// really need it.
 void download_list::paint(const style &st)
 {
   int y=0;

Modified: branches/aptitude-0.3/aptitude/src/download_screen.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/download_screen.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/download_screen.cc	Fri Sep  2 19:54:57 2005
@@ -39,18 +39,6 @@
 #include <sigc++/functors/ptr_fun.h>
 #include <generic/mut_fun.h>
 
-#define CRITICAL_ENTER	\
-  sigset_t ___signals,___oldset;\
-  sigemptyset(&___signals);	\
-  sigaddset(&___signals, SIGWINCH);\
-  sigprocmask(SIG_BLOCK, &___signals, &___oldset);\
-  assert(!sigismember(&___oldset, SIGWINCH));
-
-#define CRITICAL_EXIT	\
-  sigprocmask(SIG_SETMASK, &___oldset, &___signals);\
-  assert(sigismember(&___signals, SIGWINCH));
-
-// argh
 static void set_and_exit(bool &target, bool val)
 {
   target=val;
@@ -59,8 +47,6 @@
 
 bool download_screen::MediaChange(string Media, string Drive)
 {
-  CRITICAL_ENTER
-
   char buf[512];
 
   snprintf(buf, 512,
@@ -78,8 +64,6 @@
 			       transcode(_("Abort")),
 			       get_style("MediaChange")));
 
-  CRITICAL_EXIT
-
   vscreen_mainloop();  // Eeeeeek!  Recursive mainloop!  I'm afraid..
 
   return rval;
@@ -87,8 +71,6 @@
 
 void download_screen::IMSHit(pkgAcquire::ItemDesc &itmdesc)
 {
-  CRITICAL_ENTER
-
   downloadmap::iterator found=active_items.find(itmdesc.Owner);
 
   if(found==active_items.end())
@@ -102,13 +84,10 @@
     found->second->download_done(true);
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 void download_screen::Fetch(pkgAcquire::ItemDesc &itmdesc)
 {
-  CRITICAL_ENTER
   downloadmap::iterator found=active_items.find(itmdesc.Owner);
 
   if(found==active_items.end())
@@ -121,13 +100,10 @@
 
   vscreen_update();
   vscreen_tryupdate();
-  CRITICAL_EXIT
 }
 
 void download_screen::Done(pkgAcquire::ItemDesc &itmdesc)
 {
-  CRITICAL_ENTER
-
   downloadmap::iterator found=active_items.find(itmdesc.Owner);
   if(found==active_items.end())
     {
@@ -144,13 +120,10 @@
 
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 void download_screen::Fail(pkgAcquire::ItemDesc &itmdesc)
 {
-  CRITICAL_ENTER
   downloadmap::iterator found=active_items.find(itmdesc.Owner);
   if(found!=active_items.end())
     found->second->set_worker(NULL);
@@ -158,13 +131,10 @@
   // Nothing really to do??
   vscreen_update();
   vscreen_tryupdate();
-
-  CRITICAL_EXIT
 }
 
 bool download_screen::Pulse(pkgAcquire *Owner)
 {
-  CRITICAL_ENTER
   pkgAcquireStatus::Pulse(Owner);
 
   for(pkgAcquire::Worker *i=Owner->WorkersBegin(); i; i=Owner->WorkerStep(i))
@@ -178,22 +148,18 @@
   vscreen_update();
   vscreen_tryupdate();
 
-  CRITICAL_EXIT
   return !cancelled;
 }
 
 void download_screen::Start()
 {
-  CRITICAL_ENTER
   pkgAcquireStatus::Start();
-  CRITICAL_EXIT
 }
 
 void download_screen::Stop()
 {
   char buf[256];
 
-  CRITICAL_ENTER
   pkgAcquireStatus::Stop();
 
   snprintf(buf, 256, _("Downloaded %sB in %ss (%sB/s)."), SizeToStr(FetchedBytes).c_str(), TimeToStr(ElapsedTime).c_str(), SizeToStr(CurrentCPS).c_str());
@@ -201,8 +167,6 @@
   popup_widget(vs_dialog_ok(transcode(buf),
 			    arg(sigc::ptr_fun(vscreen_exitmain))));
 
-  CRITICAL_EXIT
-
   vscreen_mainloop();
 
   destroy();

Modified: branches/aptitude-0.3/aptitude/src/vscreen/Makefile.am
==============================================================================
--- branches/aptitude-0.3/aptitude/src/vscreen/Makefile.am	(original)
+++ branches/aptitude-0.3/aptitude/src/vscreen/Makefile.am	Fri Sep  2 19:54:57 2005
@@ -89,4 +89,4 @@
 testvscreen_SOURCES=	\
 	testvscreen.cc
 
-testvscreen_LDADD=libvscreen.a config/libconf.a
+testvscreen_LDADD=libvscreen.a config/libconf.a ../generic/libgeneric.a

Modified: branches/aptitude-0.3/aptitude/src/vscreen/curses++.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/vscreen/curses++.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/vscreen/curses++.cc	Fri Sep  2 19:54:57 2005
@@ -235,7 +235,7 @@
       struct winsize w;
       if(ioctl(fd, TIOCGWINSZ, &w)!=-1)
 	{
-	  resizeterm(w.ws_row,w.ws_col);
+	  resize_term(w.ws_row,w.ws_col);
 	  rootwin=newwin(w.ws_row, w.ws_col, 0, 0);
 	  assert(rootwin);
 	  //assert(rootwin.getmaxy()==w.ws_row);

Modified: branches/aptitude-0.3/aptitude/src/vscreen/vscreen.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/vscreen/vscreen.cc	(original)
+++ branches/aptitude-0.3/aptitude/src/vscreen/vscreen.cc	Fri Sep  2 19:54:57 2005
@@ -42,21 +42,31 @@
 #include "config/keybindings.h"
 #include "config/style.h"
 
+#include <generic/event_queue.h>
+#include <generic/threads.h>
+
 // For _()
 #include "../aptitude.h"
 
 #include <signal.h>
 
-#ifdef HAVE_LIBPTHREAD
-#include <pthread.h>
-#endif
-
 #include <assert.h>
 #include <sys/time.h>
 
-#ifdef HAVE_LIBPTHREAD
-static pthread_mutex_t vscreen_mutex;
-#endif
+#include <map>
+
+threads::recursive_mutex vscreen_mutex;
+
+inline
+threads::mutex &vscreen_get_mutex()
+{
+  return vscreen_mutex;
+}
+
+sigc::signal0<void> main_hook;
+
+
+static threads::event_queue<vscreen_event *> eventq;
 
 using namespace std;
 
@@ -67,47 +77,34 @@
 static struct sigaction oldsigcont, oldsigtstp;
 
 // Used to queue and merge update requests
-static bool needs_layout=false;
-static bool needs_update=false;
-static bool needs_cursorupdate=false;
-
-sigc::signal0<void> main_hook;
-
-struct timeout_info
-{
-  sigc::slot0<void> f;
-  int num;
-  timeval activate_time; // tells when this timeout should be triggered
-  timeout_info(const sigc::slot0<void> &_f, int _num, timeval &_activate_time)
-    :f(_f), num(_num), activate_time(_activate_time)
-  {}
+//
+// The global event queue isn't used for this so that
+// vscreen_update(); vscreen_tryupdate() works as desired.  However,
+// threading magic is used to ensure that background threads can post
+// update requests.
+struct update_state
+{
+  bool layout;
+  bool update;
+  bool cursorupdate;
+
+  update_state()
+    :layout(false), update(false), cursorupdate(false)
+  {
+  }
 };
 
-list<timeout_info> timeouts;
-// Timeouts which have been registered.  For optimal speed this should be a
-// heap, but there are other considerations...it might be necessary for callers
-// to delete arbitrary timeouts, and while maintaining heap order with
-// arbitrary deletions might be possible, it's a bit of a nuisance.  Also,
-// I don't expect this to ever hold more than a handful of values.
-//  In any event, the external interface doesn't depend on details like
-// this, so it will be fairly simple to switch to using a more efficient
-// data structure internally if this becomes a problem.  I notice that glib
-// uses something similar to this, and it's efficient enough..  Optimizing
-// stuff that doesn't need to be optimized is a waste of time.
-int ntimeouts=0;
-// Cheesy way of generating mostly-unique IDs for timeouts -- just incremented 
-// once for every timeout that gets added.
+threads::recursive_mutex pending_updates_mutex;
+update_state pending_updates;
 
-static vs_widget_ref toplevel = NULL;
-// The widget which is displayed as the root of everything
 
-//static bool resized=false;
-static void sigresize(int sig)
+vscreen_event::~vscreen_event()
 {
-  vscreen_handleresize();
-  //resized=true;
 }
 
+static vs_widget_ref toplevel = NULL;
+// The widget which is displayed as the root of everything
+
 // Cleanly shutdown (eg, restore screen settings if possible)
 //
 // Called on SIGTERM, SIGINT, SIGSEGV, SIGABRT, and SIGQUIT
@@ -190,6 +187,421 @@
   return oldw;
 }
 
+//////////////////////////////////////////////////////////////////////
+void vscreen_post_event(vscreen_event *ev)
+{
+  eventq.put(ev);
+}
+
+//////////////////////////////////////////////////////////////////////
+// Event management threads
+
+/** This thread is responsible for posting wget_wch() calls. */
+class input_thread
+{
+  class key_input_event : public vscreen_event
+  {
+    key k;
+  public:
+    key_input_event (const key &_k)
+      :k(_k)
+    {
+    }
+
+    void dispatch()
+    {
+      if(global_bindings.key_matches(k, "Refresh"))
+	vscreen_redraw();
+      else
+	toplevel->dispatch_key(k);
+    }
+  };
+
+  class mouse_input_event : public vscreen_event
+  {
+    MEVENT ev;
+
+  public:
+    mouse_input_event(const MEVENT &_ev)
+      :ev(_ev)
+    {
+    }
+
+    void dispatch()
+    {
+      if(toplevel.valid())
+	toplevel->dispatch_mouse(ev.id, ev.x, ev.y, ev.z, ev.bstate);
+    }
+  };
+
+  static input_thread instance;
+
+  static threads::mutex instance_mutex;
+  static threads::thread *instancet;
+public:
+  static void start()
+  {
+    threads::mutex::lock l(instance_mutex);
+
+    if(instancet == NULL)
+      instancet = new threads::thread(instance);
+  }
+
+  static void stop()
+  {
+    threads::mutex::lock l(instance_mutex);
+
+    if(instancet != NULL)
+      {
+	instancet->cancel();
+	instancet->join();
+	delete instancet;
+	instancet = NULL;
+      }
+  }
+
+  void operator()() const
+  {
+    // NB: use the GLOBAL getch function to avoid weirdness
+    // referencing the state of toplevel.
+    while(1)
+      {
+	wint_t wch = 0;
+	int status;
+
+	do
+	  {
+	    status = get_wch(&wch);
+	  } while(status == KEY_CODE_YES && wch == KEY_RESIZE);
+
+	key k(wch, status == KEY_CODE_YES);
+
+	if(status == ERR)
+	  return; // ???
+
+	if(wch == KEY_MOUSE)
+	  {
+	    MEVENT ev;
+	    getmouse(&ev);
+
+	    vscreen_post_event(new mouse_input_event(ev));
+	  }
+	else
+	  vscreen_post_event(new key_input_event(k));
+      }
+  }
+};
+
+threads::mutex input_thread::instance_mutex;
+threads::thread *input_thread::instancet = NULL;
+input_thread input_thread::instance;
+
+class signal_thread
+{
+  class signal_event : public vscreen_event
+  {
+    int signal;
+  public:
+    signal_event(int _signal)
+      :signal(_signal)
+    {
+    }
+
+    void dispatch()
+    {
+      switch(signal)
+	{
+	case SIGWINCH:
+	  vscreen_handleresize();
+	  break;
+	default:
+	  vscreen_exitmain();
+	  break;
+	}
+    }
+  };
+
+  static signal_thread instance;
+  static threads::thread *t;
+public:
+  static void start()
+  {
+    if(t == NULL)
+      t = new threads::thread(instance);
+  }
+
+  static void stop()
+  {
+    if(t != NULL)
+      {
+	t->cancel();
+	t->join();
+	delete t;
+	t = NULL;
+      }
+  }
+
+  void operator()() const
+  {
+    sigset_t s;
+
+    sigemptyset(&s);
+    sigaddset(&s, SIGWINCH);
+
+    while(1)
+      {
+	int signum;
+
+	int result = sigwait(&s, &signum);
+
+	if(result == 0)
+	  vscreen_post_event(new signal_event(signum));
+      }
+  }
+};
+
+signal_thread signal_thread::instance;
+threads::thread *signal_thread::t = NULL;
+
+class timeout_thread
+{
+  class SingletonViolationException
+  {
+  public:
+    string errmsg() const
+    {
+      return "Attempt to run a singleton thread twice!";
+    }
+  };
+
+  class timeout_event : public vscreen_event
+  {
+    sigc::slot0<void> slot;
+
+  public:
+    timeout_event(const sigc::slot0<void> &_slot)
+      :slot(_slot)
+    {
+    }
+
+    void dispatch()
+    {
+      slot();
+    }
+  };
+
+
+  /** Information about a single time-out. */
+  struct timeout_info
+  {
+    sigc::slot0<void> f;
+    timeval activate_time; // tells when this timeout should be triggered
+    timeout_info(const sigc::slot0<void> &_f,
+		 const timeval &_activate_time)
+      :f(_f), activate_time(_activate_time)
+    {
+    }
+
+    timeout_info()
+    {
+      activate_time.tv_sec = 0;
+      activate_time.tv_usec = 0;
+    }
+  };
+
+
+  // The set of active timeouts.
+  map<int, timeout_info> timeouts;
+
+  // A lock for the set of timeouts.
+  threads::mutex timeouts_mutex;
+
+  // A condition to be broadcast when the set of timeouts is expanded
+  // by add_timeout.
+  threads::condition timeout_added;
+
+  threads::box<bool> running;
+
+
+
+  /** Post messages about any timeouts that occurred. Should be called
+   *  with timeouts_mutex locked.
+   */
+  void check_timeouts()
+  {
+    map<int, timeout_info>::iterator i,j;
+    for(i = timeouts.begin(); i != timeouts.end(); i = j)
+      {
+	j = i;
+	j++;
+	timeval result,curtime;
+	gettimeofday(&curtime, 0);
+
+	if(timeval_subtract(&result, &i->second.activate_time, &curtime) == 1 ||
+	   result.tv_sec == 0 && result.tv_usec <= 10)
+	  {
+	    vscreen_post_event(new timeout_event(i->second.f));
+	    timeouts.erase(i);
+	  }
+      }
+  }
+
+  /** timeouts_mutex should be locked when this is called.
+   *
+   *  \return the time at which the first active timeout should trigger.
+   */
+  bool first_timeout(timeval &tv_out)
+  {
+    bool found_one = false;
+    timeval mintime;
+    mintime.tv_sec = INT_MAX/1000;
+    mintime.tv_usec = (INT_MAX % 1000) * 1000;
+
+    timeval curtime;
+    gettimeofday(&curtime, 0);
+
+    map<int, timeout_info>::iterator i,j;
+    for(i = timeouts.begin(); i != timeouts.end(); i = j)
+      {
+	j = i;
+	j++;
+	timeval diff;
+	if(timeval_subtract(&diff, &i->second.activate_time, &curtime) == 1 ||
+	   diff.tv_sec == 0 && diff.tv_usec <= 10)
+	  return 0;
+	else
+	  {
+	    if(diff.tv_sec < mintime.tv_sec ||
+	       (diff.tv_sec == mintime.tv_sec && diff.tv_usec < mintime.tv_usec))
+	      {
+		found_one = true;
+		mintime.tv_sec = diff.tv_sec;
+		mintime.tv_usec = diff.tv_usec;
+	      }
+	  }
+      }
+    if(found_one)
+      {
+	tv_out = mintime;
+	return true;
+      }
+    else
+      return false;
+  }
+
+
+  timeout_thread(const timeout_thread &other);
+  timeout_thread &operator=(const timeout_thread &other);
+
+  timeout_thread()
+    : running(false)
+  {
+  }
+
+  // The global instance; this is a Singleton.
+  static timeout_thread instance;
+
+  // Unfortunately, technical considerations in the threading code
+  // mean that the actual thread object is expected to be copyable.
+  // Hence this proxy:
+  class timeout_proxy
+  {
+  public:
+    void operator()() const
+    {
+      get_instance()();
+    }
+  };
+public:
+  static timeout_thread &get_instance()
+  {
+    return instance;
+  }
+
+  static void start()
+  {
+    threads::thread t(timeout_proxy());
+  }
+
+  void operator()()
+  {
+    bool was_running = running.take();
+    running.put(true);
+
+    if(was_running)
+      throw SingletonViolationException();
+
+    threads::mutex::lock l(timeouts_mutex);
+
+    while(1)
+      {
+	timeval next_timeout;
+
+	if(first_timeout(next_timeout))
+	  {
+	    timespec until;
+	    until.tv_sec = next_timeout.tv_sec;
+	    until.tv_nsec = next_timeout.tv_usec * 1000;
+
+	    timeout_added.timed_wait(l, until);
+
+	    // Probably don't need to do this, but it won't hurt.
+	    check_timeouts();
+	  }
+	else
+	  timeout_added.wait(l);
+      }
+  }
+
+  /** Add a timeout to the set of active timeouts.
+   *
+   *  \param slot a callback to activate when the timeout triggers
+   *  \param msecs the number of milliseconds in which to activate the
+   *               timeout.
+   *  \return the ID number of the new timeout; can be used later to
+   *          remove the timeout before it triggers.
+   */
+  int add_timeout(const sigc::slot0<void> &slot, int msecs)
+  {
+    threads::mutex::lock l(timeouts_mutex);
+
+    timeval activate_time;
+    gettimeofday(&activate_time, 0);
+    activate_time.tv_sec  += msecs/1000;
+    activate_time.tv_usec += (msecs%1000)*1000;
+    while(activate_time.tv_usec > 1000 * 1000)
+      // Should only run through once
+      {
+	activate_time.tv_sec++;
+	activate_time.tv_usec -= 1000 * 1000;
+      }
+
+    // Since the timeouts are sorted by ID, the back element is the
+    // maximum ID in use.
+    int rval;
+
+    if(timeouts.empty())
+      rval = 0;
+    else
+      rval = timeouts.rbegin()->first + 1;
+
+    timeouts[rval] = timeout_info(slot, activate_time);
+
+    timeout_added.wake_all();
+
+    return rval;
+  }
+
+  void del_timeout(int id)
+  {
+    threads::mutex::lock l(timeouts_mutex);
+
+    timeouts.erase(id);
+  }
+};
+
+timeout_thread timeout_thread::instance;
+
 void vscreen_init()
 {
   keybinding upkey, downkey, leftkey, rightkey, quitkey, homekey, endkey;
@@ -336,15 +748,21 @@
   if(toplevel.valid())
     vscreen_settoplevel(toplevel);
 
-  pthread_mutexattr_t vscreen_mutex_attr;
-  pthread_mutexattr_init(&vscreen_mutex_attr);
-  pthread_mutexattr_settype(&vscreen_mutex_attr, PTHREAD_MUTEX_RECURSIVE);
 
-  pthread_mutex_init(&vscreen_mutex, &vscreen_mutex_attr);
+  vscreen_install_sighandlers();
 
-  pthread_mutexattr_destroy(&vscreen_mutex_attr);
 
-  vscreen_install_sighandlers();
+  // Block WINCH so the signal_thread can pick it up.
+  sigset_t signals;
+  sigemptyset(&signals);
+  sigaddset(&signals, SIGWINCH);
+  sigprocmask(SIG_BLOCK, &signals, NULL);
+
+
+
+  input_thread::start();
+  signal_thread::start();
+  timeout_thread::start();
 }
 
 void vscreen_install_sighandlers()
@@ -364,70 +782,20 @@
   vscreen_redraw();
 }
 
-void vscreen_checktimeouts()
+class try_update_event : public vscreen_event
 {
-  list<timeout_info>::iterator i,j;
-  for(i=timeouts.begin(); i!=timeouts.end(); i=j)
-    {
-      j=i;
-      j++;
-      timeval result,curtime;
-      gettimeofday(&curtime, 0);
-
-      if(timeval_subtract(&result, &i->activate_time, &curtime)==1 ||
-	 result.tv_sec==0 && result.tv_usec<=10)
-	// Fudge things a little.. (good idea or not?)
-	{
-	  i->f();
-	  timeouts.erase(i);
-	}
-    }
-
-  vscreen_tryupdate();
-}
-
-int vscreen_mindelay()
-  // Returns the number of milliseconds to delay for the first timeout. (-1 for
-  // infinite delay)
-  // If a timeout should be done now, returns 0.
-{
-  bool found_one=false;
-  timeval mintime;
-  mintime.tv_sec=INT_MAX/1000;
-  mintime.tv_usec=(INT_MAX%1000)*1000;
-
-  timeval curtime;
-  gettimeofday(&curtime, 0);
-
-  list<timeout_info>::iterator i,j;
-  for(i=timeouts.begin(); i!=timeouts.end(); i=j)
-    {
-      j=i;
-      j++;
-      timeval diff;
-      if(timeval_subtract(&diff, &i->activate_time, &curtime)==1 ||
-	 diff.tv_sec==0 && diff.tv_usec<=10)
-	return 0;
-      else
-	{
-	  if(diff.tv_sec<mintime.tv_sec ||
-	     (diff.tv_sec==mintime.tv_sec && diff.tv_usec<mintime.tv_usec))
-	  {
-	    found_one=true;
-	    mintime.tv_sec=diff.tv_sec;
-	    mintime.tv_usec=diff.tv_usec;
-	  }
-	}
-    }
-  if(found_one)
-    return mintime.tv_sec*1000+mintime.tv_usec/1000;
-  else
-    return -1;
-}
+public:
+  void dispatch()
+  {
+    vscreen_tryupdate();
+  }
+};
 
 void vscreen_updatecursor()
 {
-  needs_cursorupdate=true;
+  threads::mutex::lock l(pending_updates_mutex);
+
+  pending_updates.cursorupdate=true;
 }
 
 void vscreen_updatecursornow()
@@ -440,14 +808,16 @@
     }
   else
     toplevel->win.leaveok(true);
-
-  needs_cursorupdate=false;
 }
 
 void vscreen_update()
 {
-  needs_update=true;
-  needs_cursorupdate=true;
+  threads::mutex::lock l(pending_updates_mutex);
+
+  pending_updates.update=true;
+  pending_updates.cursorupdate=true;
+
+  vscreen_post_event(new try_update_event);
 }
 
 void vscreen_updatenow()
@@ -461,183 +831,95 @@
 
       doupdate();
     }
-  needs_update=false;
+}
+
+void vscreen_queuelayout()
+{
+  threads::mutex::lock l(pending_updates_mutex);
+
+  pending_updates.layout = true;
+  pending_updates.update = true;
+  pending_updates.cursorupdate = true;
+
+  vscreen_post_event(new try_update_event);
 }
 
 void vscreen_layoutnow()
 {
   toplevel->do_layout();
-  needs_layout=false;
 }
 
 void vscreen_tryupdate()
 {
-  if(needs_layout)
+  threads::mutex::lock l(pending_updates_mutex);
+
+  update_state needs = pending_updates;
+
+  if(needs.layout)
     vscreen_layoutnow();
 
-  if(needs_update)
+  if(needs.update)
     vscreen_updatenow();
 
-  if(needs_cursorupdate)
+  if(needs.cursorupdate)
     {
       vscreen_updatecursornow();
       refresh();
     }
+
+  // \todo This appears to just paper over sloppiness -- screen update
+  // routines shouldn't be queuing more updates!
+  pending_updates = update_state();
 }
 
 bool vscreen_poll()
-  // FIXME: should I take the global lock?  Need to think about this before
-  //       using libvscreen for threaded programming
 {
   bool rval=false;
 
-  sigset_t signals,prevsignals;
-
-  sigemptyset(&signals);
-  sigaddset(&signals, SIGWINCH);
+  vscreen_event *ev = NULL;
 
-  if(toplevel.valid())
+  while(eventq.try_get(ev))
     {
-      wint_t wch = 0;
-      int status;
-
-      do
-	{
-	  toplevel->win.nodelay(true);
-	  status = toplevel->win.get_wch(&wch);
-	  toplevel->win.nodelay(false);
-	} while(status == KEY_CODE_YES && wch == KEY_RESIZE);
-
-      key k(wch, status == KEY_CODE_YES);
-
-      sigprocmask(SIG_BLOCK, &signals, &prevsignals);
-
-      if(status != ERR)
-	{
-	  rval = true;
-
-	  if(wch == KEY_MOUSE)
-	    {
-	      MEVENT ev;
-	      getmouse(&ev);
-
-	      if(toplevel.valid())
-		{
-		  toplevel->dispatch_mouse(ev.id, ev.x, ev.y, ev.z, ev.bstate);
-		  main_hook();
-		  vscreen_tryupdate();
-		}
-	    }
-	  else if(global_bindings.key_matches(k, "Refresh"))
-	    vscreen_redraw();
-	  else if(toplevel.valid())
-	    {
-	      toplevel->dispatch_key(k);
-	      main_hook();
-	      vscreen_tryupdate();
-	    }
-	}
+      rval = true;
+      ev->dispatch();
+      delete ev;
     }
 
-  vscreen_checktimeouts();
-
-  sigprocmask(SIG_SETMASK, &signals, &prevsignals);
+  main_hook();
 
   return rval;
 }
 
 void vscreen_mainloop(int synch)
-  // FIXME: can I restructure things so that this just calls vscreen_poll
-  // repeatedly?  It'd be a lot cleaner..
 {
   static int main_level=0;
-  // HACK -- basically, delayed deletion avoids crashes by guaranteeing that
-  // things only get deleted in the main loop.  But this breaks if sub-main-loops
-  // delete them.  Solution: don't delete things in sub-main-loops..
 
   main_level++;
 
-  sigset_t signals;
-
-  sigemptyset(&signals);
-  sigaddset(&signals, SIGWINCH);
-
-  signal(SIGWINCH, sigresize);
-
-  vscreen_acquirelock();
-
-  // Do this first, so any pending updates from initialization routines get
-  // flushed.
-  vscreen_tryupdate();
+  threads::mutex::lock l(vscreen_get_mutex());
 
   while(!should_exit && toplevel.valid())
     {
-      sigset_t prevsignals;
-
-      assert(toplevel->get_win());
-
-      int prevtimeout=toplevel->timeout(vscreen_mindelay());
-      vs_widget_ref curwidget=toplevel;
-      // Note that something bad might happen while we don't have the mutex..
-      // OTOH, it's perfectly okay to dispatch the character to an unexpected
-      // location; I just want to be sure that I reset the timeout of the
-      // correct widget. (??)
-
-      wint_t wch;
-      int result;
+      l.release();
 
-      do
-	{
-	  vscreen_releaselock();
-	  result=curwidget->get_win().get_wch(&wch);
-	  vscreen_acquirelock();
-	} while(result == KEY_CODE_YES && wch == KEY_RESIZE);
-
-      key k = key(wch, result == KEY_CODE_YES);
-
-      // used when asynchronous resizing was dangerous
-      //if(resized)
-      //	{
-      //  resized=false;
-      //  vscreen_handleresize();
-      //}
+      vscreen_event *ev = eventq.get();
 
-      curwidget->timeout(prevtimeout);
+      l.acquire();
 
-      sigprocmask(SIG_BLOCK, &signals, &prevsignals);
-      // Oddly enough, the part I want to *protect* from signals is my own code
-      // -- the ncurses code above is actually graceful about this situation!
-
-      vscreen_checktimeouts();
-
-      if(wch==KEY_MOUSE)
-	{
-	  MEVENT ev;
-	  getmouse(&ev);
+      ev->dispatch();
+      delete ev;
 
-	  if(toplevel.valid())
-	    {
-	      toplevel->dispatch_mouse(ev.id, ev.x, ev.y, ev.z, ev.bstate);
-	      main_hook();
-	      vscreen_tryupdate();
-	    }
-	}
-      else if(global_bindings.key_matches(k, "Refresh"))
-	vscreen_redraw();
-      else if(result!=ERR && toplevel.valid())
+      while(eventq.try_get(ev))
 	{
-	  toplevel->dispatch_key(k);
-	  main_hook();
-	  vscreen_tryupdate();
+	  ev->dispatch();
+	  delete ev;
 	}
 
-      sigprocmask(SIG_SETMASK, &prevsignals, NULL);
+      main_hook();
     }
 
   should_exit=false;
 
-  vscreen_releaselock();
-
   main_level--;
 }
 
@@ -648,6 +930,9 @@
 
 void vscreen_suspend_without_signals()
 {
+  input_thread::stop();
+  signal_thread::stop();
+
   if(toplevel.valid())
     toplevel->set_owner_window(NULL, 0, 0, 0, 0);
 
@@ -681,6 +966,11 @@
   toplevel = NULL;
 
   vscreen_suspend();
+
+  // Discard all remaining events.
+  vscreen_event *ev = NULL;
+  while(eventq.try_get(ev))
+    delete ev;
 }
 
 void vscreen_resume()
@@ -702,17 +992,15 @@
     }
   else
     refresh();
-}
 
-void vscreen_queuelayout()
-{
-  needs_layout=true;
-  needs_update=true;
-  needs_cursorupdate=true;
+  input_thread::start();
+  signal_thread::start();
 }
 
 void vscreen_redraw()
 {
+  threads::mutex::lock l(pending_updates_mutex);
+
   if(toplevel.valid())
     {
       toplevel->focussed();
@@ -725,52 +1013,23 @@
       doupdate();
       toplevel->get_win().clearok(false);
     }
+
+  // For reasons that aren't entirely clear, running a tryupdate()
+  // right after a resize causes stuff to break -- but since resizing
+  // triggers a redraw, the tryupdate() shouldn't be necessary anyway.
+  // Suppress any pending updates after redrawing:
+  pending_updates = update_state();
 }
 
 int vscreen_addtimeout(const sigc::slot0<void> &slot, int msecs)
 {
-  if(msecs<0)
+  if(msecs < 0)
     return -1;
 
-  int rval=ntimeouts++;
-  timeval activate_time;
-  gettimeofday(&activate_time, 0);
-  activate_time.tv_sec+=msecs/1000;
-  activate_time.tv_usec+=(msecs%1000)*1000;
-  while(activate_time.tv_usec>1000*1000)
-    // Should only run through once
-    {
-      activate_time.tv_sec++;
-      activate_time.tv_usec-=1000*1000;
-    }
-
-  timeouts.push_back(timeout_info(slot, rval, activate_time));
-  return rval;
+  return timeout_thread::get_instance().add_timeout(slot, msecs);
 }
 
 void vscreen_deltimeout(int num)
 {
-  for(list<timeout_info>::iterator i=timeouts.begin(); i!=timeouts.end(); i++)
-    {
-      if(i->num==num)
-	{
-	  timeouts.erase(i);
-	  return;
-	}
-    }
-}
-
-void vscreen_acquirelock()
-{
-#ifdef HAVE_LIBPTHREAD
-  pthread_mutex_lock(&vscreen_mutex);
-#endif
-}
-
-void vscreen_releaselock()
-{
-#ifdef HAVE_LIBPTHREAD
-  pthread_mutex_unlock(&vscreen_mutex);
-#endif
+  timeout_thread::get_instance().del_timeout(num);
 }
-

Modified: branches/aptitude-0.3/aptitude/src/vscreen/vscreen.h
==============================================================================
--- branches/aptitude-0.3/aptitude/src/vscreen/vscreen.h	(original)
+++ branches/aptitude-0.3/aptitude/src/vscreen/vscreen.h	Fri Sep  2 19:54:57 2005
@@ -31,6 +31,16 @@
 
 #include "ref_ptr.h"
 
+/** An event in the global event queue.  Events are dispatched
+ *  serially by the main loop.
+ */
+class vscreen_event
+{
+public:
+  virtual void dispatch() = 0;
+  virtual ~vscreen_event();
+};
+
 class vscreen_widget;
 
 void vscreen_init();
@@ -48,8 +58,10 @@
  */
 ref_ptr<vscreen_widget> vscreen_settoplevel(const ref_ptr<vscreen_widget> &widget);
 
+/** Posts a request to recalculate every widget's layout and update
+ *  the screen.  May be called from any thread.
+ */
 void vscreen_queuelayout();
-// Queues a re-layout of all widgets (needed when something has to be resized)
 
 void vscreen_layoutnow();
 // Lays out all widgets immediately.
@@ -60,12 +72,22 @@
 // Enters a loop, calling getch() over and over and over again..
 // A valid vscreen must be currently displayed.
 
+/** Post the given event to the main event queue.  When the event
+ *  comes off the queue, its dispatch method will be invoked and it
+ *  will immediately be destroyed.
+ *
+ *  This method is thread-safe and is the main mechanism by which
+ *  other threads should communicate with the main thread.
+ */
+void vscreen_post_event(vscreen_event *ev);
+
+/** Dispatch any events in the event queue.  This is deprecated in
+ *  favor of the more reliable approach of using threads and
+ *  post_event.
+ *
+ *  \return \b true if pending input was found.
+ */
 bool vscreen_poll();
-//  Checks for pending input and dispatches any it finds and calls any needed
-// timeouts.  Can be called from within a input-handling or timeout routine
-// called by vscreen_main(); there (should!) be no reentrance issues.
-//
-//  Returns true if pending input was found.
 
 void vscreen_exitmain();
 // Exits the main loop.
@@ -73,15 +95,16 @@
 void vscreen_redraw();
 // Redraws the screen completely from scratch
 
+/** Posts a request to redraw the screen; may be called from any thread. */
 void vscreen_update();
-// Redraws the screen; like display(); refresh(); except that it will handle
-// overlapped widgets properly
 
+/** Executes any pending draws or redraws. */
 void vscreen_tryupdate();
-// Handles any pending draws/redraws.
 
+/** Posts a request to update the cursor location; may be called from
+ *  any thread.
+ */
 void vscreen_updatecursor();
-// Just updates the cursor status
 
 /** Hides all widgets and suspends Curses operation until
  *  vscreen_resume() is called.  In addition, sets SIGCONT and SIGTSTP
@@ -116,32 +139,18 @@
 // Does anything needed to handle a window resize event.
 // FIXME: I --think-- that this is now redundant
 
-//  Thread safety routines
-//  BE VERY CAREFUL WHEN USING THESE!
-//
-//  The threading system used by vscreen is very simple: there's one lock which
-// is acquired before any vscreen-using activity takes place.  Normally
-// programs never have to worry about this, as the main loop always acquires
-// the lock before executing any callback code.  HOWEVER, if you wish to
-// write a thread running in parallel to the main one which uses vscreens, it
-// is imperative that you use vscreen_acquirelock() and vscreen_releaselock()
-// INTELLIGENTLY!  Note also that you should be VERY CAREFUL about deleting
-// vscreens from within threads other than the main loop (read: DON'T DO IT)
-// as the main loop may hold a reference to the vscreen that it's reading input
-// from!
-//
-//  One more note: treat vscreen_poll() as you would any other function; that
-// is, BE SURE that you hold the global lock before entering it!
-
-void vscreen_acquirelock();
-// Acquire the global lock
-
-void vscreen_releaselock();
-// Release it.
+//  Return a mutex that is held whenever the main loop is processing
+//  events or drawing the screen.  The mutex can be held by other
+//  threads that want to do their own drawing or processing, although
+//  this is highly discouraged (use the event posting system instead
+//  to run code in the main loop).
+namespace threads { class mutex; }
+threads::mutex &vscreen_get_mutex();
 
 extern sigc::signal0<void> main_hook;
-// Called right after we finish handling input in the mainloop.  Can be used
-// (eg) to insert extra actions to be performed after all user-input (aptitude
-// uses this to check for apt errors and pop up a message about them)
+// Called right after we finish handling input in the mainloop.  Can
+// be used (eg) to insert extra actions to be performed after all
+// user-input (aptitude uses this to check for apt errors and pop up a
+// message about them)
 
 #endif



More information about the Aptitude-svn-commit mailing list