Why can I deadlock in a single-threaded FLTK application?

Question

I have a supposedly single-threaded FLTK application with a popup menu, created with Fluid. I have a class that sub-classes Fl_Gl_Window and implements a handle() method. The handle() method calls a function that creates a popup window on right-click. I have a long operation that I do for one of the menu items. My application has created a second thread for some other purpose. I use locks to protect some critical sections between my main thread and that second thread. In particular, doLongOperation() uses the locks.

My problem is that I can popup the menu twice and run doLongOperation() twice, and it then deadlocks with itself, hanging the application. Why doesn't the first doLongOperation() stall the GUI and prevent me from starting doLongOperation() a second time?

I can avoid the problem with a flag that I use to disable the offending menu item, but I would like to understand why it is possible in the first place.

Here's the code, abbreviated of course. Hopefully I've included all the relevant bits.

class MyClass {
  void doLongOperation();
};

class MyApplication : public MyClass {
  MyApplication();
  void run();
  void popup_menu();
};

void MyClass::doLongOperation()
{
   this->enterCriticalSection();
   // stuff
   // EDIT
   // @vladr I did leave out a relevant bit.
   // Inside this critical section, I was calling Fl::check().
   // That let the GUI handle a new popup and dispatch a new
   // doLongOperation() which is what lead to deadlock.
   // END EDIT
   this->leaveCriticalSection();
} 

MyApplication::MyApplication() : MyClass() 
{
  // ...
  { m_mainWindowPtr = new Fl_Double_Window(820, 935, "Title");
    m_mainWindowPtr->callback((Fl_Callback*)cb_m_mainWindowPtr, (void*)(this));
    { m_wireFrameViewPtr = new DerivedFrom_Fl_Gl_Window(10, 40, 800, 560);
      // ...
    }
    m_mainWindowPtr->end();
  } // Fl_Double_Window* m_mainWindowPtr

m_wireFrameViewPtr->setInteractive();

m_mainWindowPtr->position(7,54);
m_mainWindowPtr->show(1, &(argv[0]));

Fl::wait();
}

void MyApplication::run() {
  bool keepRunning = true;
  while(keepRunning) {

  m_wireFrameViewPtr->redraw();
  m_wireFrameView2Ptr->redraw();

  MyClass::Status result = this->runOneIteration();
  switch(result) {
  case DONE: keepRunning = false; break;
  case NONE: Fl::wait(0.001); break;
  case MORE: Fl::check(); break;
  default: keepRunning = false;
  }

}

void MyApplication::popup_menu() {
  Fl_Menu_Item *rclick_menu;


  int longOperationFlag = 0;
  // To avoid the deadlock I can set the flag when I'm "busy".
  //if (this->isBusy()) longOperationFlag = FL_MENU_INACTIVE;

  Fl_Menu_Item single_rclick_menu[] = {
     { "Do long operation", 0, 0, 0, longOperationFlag },
     // etc. ...
     { 0 }
  };

  // Define multiple_rclick_menu...

  if (this->m_selectedLandmarks.size() == 1) rclick_menu = single_rclick_menu;
  else rclick_menu = multiple_rclick_menu;

  const Fl_Menu_Item *m = rclick_menu->popup(Fl::event_x(), Fl::event_y(), 0, 0, 0);

  if (!m) return;


  if (strcmp(m->label(), "Do long operation") == 0) {
    this->doLongOperation();
    return;
  }

  // Etc.

}

Answer 1

By any chance, does your doLongOperation do anything that might need the message pump (or APCs, some windows' file API uses these underneath) to be running (assuming you see this behavior on Windows)? For example, if the doLongOperation tries to update the GUI that uses SendMessage underneath, you will get the deadlock even in a single-threaded scenario.

Also, does another thread have the critical section already claimed? You should be able to break in the debugger during the hang and hopefully see who is waiting on what.

Answer 2

Make sure that you are not calling Fl::wait(...) from more than one thread. Am I right in inferring from your code that run() executes in its own thread?

A first call to Fl::wait(), e.g. from the main thread, would catch and process the first right-click (blocking, as expected, while the first call to doLongOperation() proceeds); in the meantime, a second thread's calls to e.g. Fl::wait(timeout)/Fl::check() would continue to refresh the display -- and will intercept (and service) the second right-click, calling handle() (in the second thread) while the first long operation is still trudging along. This would give the appearance of a deadlock, although I expect that the UI would resume redrawing (through the second thread) when both long operations complete.

Validate the above by logging the current thread ID inside popup_menu().

You should pick a single thread to call Fl::wait(...) in a loop, and you should not block that loop -- spawn any non-modal or non-UI tasks as separate threads. I.e. when popup_menu() gets called, kick off the long operation in its own thread; if popup_menu() is triggered (again) while the long operation thread is still running, either mark the popup menu item as disabled (analogous to your workaround), or simply signal the long operation thread to restart with a new parameter.

Cheers, V.