[PATCH] fix "key leak"

ManMower manmower at signalmarketing.com
Tue Feb 11 14:31:24 EST 2003


this patch comes with a warning - there are quite probably cases I've not
properly handled that will leave the keyboard in a grabbed state. :/

this patch addresses the "key leak" issue - if you hold down a grabbed
key (say, alt-tab with linear cycling), sometimes the key won't properly
be grabbed by openbox and will end up going to the client.

this is fixed by doing a synch keyboard grab on all the first level
keybindings, and switching to an exclusive XGrabKeyboard before allowing
any events to continue.

it also doesn't change the grabbed keys at all if a single key long
"chain" fires.  this should cut down on a bit of extraneous server
traffic.  (this alone should be enough to get rid of the simplest case of
the key leak)

another addition is a function to ungrab all grabbed keys at once with a
single XUngrabKey.
-------------- next part --------------
? ABOUT-NLS
Index: otk/display.cc
===================================================================
RCS file: /cvs/cvsroot/openbox/otk/display.cc,v
retrieving revision 1.37
diff -p -u -r1.37 display.cc
--- otk/display.cc	2003/02/09 23:07:53	1.37
+++ otk/display.cc	2003/02/11 19:12:08
@@ -300,4 +300,9 @@ void Display::ungrabKey(unsigned int key
                grab_window);
 }
 
+void Display::ungrabAllKeys(Window grab_window) const
+{
+  XUngrabKey(_display, AnyKey, AnyModifier, grab_window);
+}
+
 }
Index: otk/display.hh
===================================================================
RCS file: /cvs/cvsroot/openbox/otk/display.hh,v
retrieving revision 1.21
diff -p -u -r1.21 display.hh
--- otk/display.hh	2003/01/29 08:50:30	1.21
+++ otk/display.hh	2003/02/11 19:12:08
@@ -133,6 +133,7 @@ public:
                bool allow_scroll_lock) const;
   void ungrabKey(unsigned int keycode, unsigned int modifiers,
                  Window grab_window) const;
+  void ungrabAllKeys(Window grab_window) const;
 };
 
 }
Index: src/bindings.cc
===================================================================
RCS file: /cvs/cvsroot/openbox/src/bindings.cc,v
retrieving revision 1.72
diff -p -u -r1.72 bindings.cc
--- src/bindings.cc	2003/02/09 23:07:54	1.72
+++ src/bindings.cc	2003/02/11 19:12:10
@@ -145,7 +145,8 @@ Bindings::Bindings()
   : _curpos(&_keytree),
     _resetkey(0,0),
     _timer((otk::Timer *) 0),
-    _keybgrab_callback(0, 0)
+    _keybgrab_callback(0, 0),
+    _grabbed(0)
 {
 //  setResetKey("C-g"); // set the default reset key
 }
@@ -155,7 +156,11 @@ Bindings::~Bindings()
 {
   if (_timer)
     delete _timer;
-
+  if (_grabbed) {
+    _grabbed = 0;
+    XUngrabKeyboard(**otk::display, CurrentTime);
+  }
+  grabKeys(false);
   removeAllKeys();
   //removeAllButtons(); // this is done by each client as they are unmanaged
   removeAllEvents();
@@ -323,7 +328,6 @@ static void remove_branch(KeyBindingTree
   }
 }
 
-
 void Bindings::removeAllKeys()
 {
   grabKeys(false);
@@ -341,28 +345,22 @@ void Bindings::grabKeys(bool grab)
     Screen *sc = openbox->screen(i);
     if (!sc) continue; // not a managed screen
     Window root = otk::display->screenInfo(i)->rootWindow();
-
-    KeyBindingTree *p = _curpos->first_child;
+    if (!grab) {
+      otk::display->ungrabAllKeys(root);
+      continue;
+    }
+    KeyBindingTree *p = _keytree.first_child;
     while (p) {
-      if (grab) {
-        otk::display->grabKey(p->binding.key, p->binding.modifiers,
-                                root, false, GrabModeAsync, GrabModeAsync,
-                                false);
-      }
-      else
-        otk::display->ungrabKey(p->binding.key, p->binding.modifiers,
-                                  root);
+      otk::display->grabKey(p->binding.key, p->binding.modifiers,
+                              root, false, GrabModeAsync, GrabModeSync,
+                              false);
       p = p->next_sibling;
     }
 
     if (_resetkey.key)
-      if (grab)
-        otk::display->grabKey(_resetkey.key, _resetkey.modifiers,
-                                root, false, GrabModeAsync, GrabModeAsync,
-                                false);
-      else
-        otk::display->ungrabKey(_resetkey.key, _resetkey.modifiers,
-                                  root);
+      otk::display->grabKey(_resetkey.key, _resetkey.modifiers,
+                              root, false, GrabModeAsync, GrabModeSync,
+                              false);
   }
 }
 
@@ -421,12 +419,13 @@ void Bindings::fireKey(int screen, unsig
     KeyData data(screen, c, time, modifiers, key, action);
     _keybgrab_callback.fire(&data);
   }
-  
+
   // KeyRelease events only occur during keyboard grabs
   if (action == KeyAction::Release) return;
     
   if (key == _resetkey.key && modifiers == _resetkey.modifiers) {
     resetChains(this);
+    XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
   } else {
     KeyBindingTree *p = _curpos->first_child;
     while (p) {
@@ -437,18 +436,23 @@ void Bindings::fireKey(int screen, unsig
           _timer = new otk::Timer(5000, // 5 second timeout
                                   (otk::Timer::TimeoutHandler)resetChains,
                                   this);
-          // grab the server here to make sure no key presses get missed
-          otk::display->grab();
-          grabKeys(false);
-          _curpos = p;
-          grabKeys(true);
-          otk::display->ungrab();
+          if (!_grabbed && !_keybgrab_callback.callback) {
+            Window root = otk::display->screenInfo(screen)->rootWindow();
+            //grab should never fail because we should have a sync grab at 
+            //this point
+            XGrabKeyboard(**otk::display, root, 0, GrabModeAsync, 
+                          GrabModeSync, CurrentTime);
+            _grabbed = 1;
+            grabKeys(false);
+          }
+          XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
         } else {
           Client *c = openbox->focusedClient();
           KeyData data(screen, c, time, modifiers, key, action);
           KeyCallbackList::iterator it, end = p->callbacks.end();
           for (it = p->callbacks.begin(); it != end; ++it)
             it->fire(&data);
+          XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
           resetChains(this);
         }
         break;
@@ -463,13 +467,19 @@ void Bindings::resetChains(Bindings *sel
   if (self->_timer) {
     delete self->_timer;
     self->_timer = (otk::Timer *) 0;
+  }
+  /* if we're not at the root, we've turned off the grabbed keys
+     and we're protected by an exclusive keyboard grab
+     else we've still got the right grabs in place */
+  if (self->_curpos != &self->_keytree) {
+    self->_curpos = &self->_keytree;
+    self->grabKeys(true);
+  }
+  if (self->_grabbed) {
+    self->_grabbed = 0;
+    if (!self->_keybgrab_callback.callback)
+      XUngrabKeyboard(**otk::display, CurrentTime);
   }
-  // grab the server here to make sure no key pressed go missed
-  otk::display->grab();
-  self->grabKeys(false);
-  self->_curpos = &self->_keytree;
-  self->grabKeys(true);
-  otk::display->ungrab();
 }
 
 
Index: src/bindings.hh
===================================================================
RCS file: /cvs/cvsroot/openbox/src/bindings.hh,v
retrieving revision 1.32
diff -p -u -r1.32 bindings.hh
--- src/bindings.hh	2003/02/05 15:38:29	1.32
+++ src/bindings.hh	2003/02/11 19:12:10
@@ -116,7 +116,9 @@ private:
   EventCallbackList _eventlist[EventAction::NUM_EVENT_ACTION];
 
   KeyCallbackData _keybgrab_callback;
-  
+
+  bool _grabbed;
+
 public:
   //! Initializes an Bindings object
   Bindings();


More information about the openbox mailing list