ilpincy / argos3

A parallel, multi-engine simulator for heterogeneous swarm robotics
http://www.argos-sim.info/
267 stars 121 forks source link

Solution to the "QObject::disconnect: Unexpected null parameter" warning #50

Open allsey87 opened 6 years ago

allsey87 commented 6 years ago

This is low priority, but there is a small bug in the Lua interface that causes the QObject::disconnect: Unexpected null parameter error. As shown in the backtrace I prepared, this results from trying to disconnect the Qt signals from m_pcLuaVariableTree->model(), which is already NULL at the time of the call to disconnect.

void CQTOpenGLLuaMainWindow::HandleEntityDeselection(size_t) {
   disconnect(&(m_pcMainWindow->GetOpenGLWidget()), SIGNAL(StepDone(int)),
              m_pcLuaVariableTree->model(), SLOT(Refresh(int)));
   disconnect(m_pcMainWindow, SIGNAL(ExperimentReset()),
              m_pcLuaVariableTree->model(), SLOT(Refresh()));
   disconnect(m_pcLuaVariableTree->model(), SIGNAL(modelReset()),
              this, SLOT(VariableTreeChanged()));
   m_pcLuaVariableDock->hide();
   delete m_pcLuaVariableTree->model();
   m_pcLuaVariableTree->setModel(NULL);
   disconnect(&(m_pcMainWindow->GetOpenGLWidget()), SIGNAL(StepDone(int)),
              m_pcLuaFunctionTree->model(), SLOT(Refresh(int)));
   disconnect(m_pcMainWindow, SIGNAL(ExperimentReset()),
              m_pcLuaFunctionTree->model(), SLOT(Refresh()));
   disconnect(m_pcLuaFunctionTree->model(), SIGNAL(modelReset()),
              this, SLOT(FunctionTreeChanged()));
   m_pcLuaFunctionDock->hide();
   delete m_pcLuaFunctionTree->model();
   m_pcLuaFunctionTree->setModel(NULL);
}

This occurs only when deselecting a non-controllable entity since m_pcLuaVariableTree->model() is never set due to the if condition below that only sets the model if the selected entity in void CQTOpenGLLuaMainWindow::HandleEntitySelection(size_t un_index) is controllable.

if(bFound &&
   m_vecControllers[m_unSelectedRobot]->GetLuaState() != NULL) {
   CQTOpenGLLuaStateTreeVariableModel* pcVarModel =
      new CQTOpenGLLuaStateTreeVariableModel(m_vecControllers[m_unSelectedRobot]->GetLuaState(),
                                             false,
                                             m_pcLuaVariableTree);
   pcVarModel->Refresh();
   connect(&(m_pcMainWindow->GetOpenGLWidget()), SIGNAL(StepDone(int)),
           pcVarModel, SLOT(Refresh(int)));
   connect(m_pcMainWindow, SIGNAL(ExperimentReset()),
           pcVarModel, SLOT(Refresh()));
   connect(pcVarModel, SIGNAL(modelReset()),
           this, SLOT(VariableTreeChanged()),
           Qt::QueuedConnection);
   m_pcLuaVariableTree->setModel(pcVarModel);
   m_pcLuaVariableTree->setRootIndex(pcVarModel->index(0, 0));
   m_pcLuaVariableTree->expandAll();
   ...
}

There are different solutions to this problem, for example, a quick hack would just check if the entity was controllable before calling disconnect. However, I think the most problematic line is m_vecControllers[m_unSelectedRobot]->GetLuaState() which seems to assume that the select robot index has a controller in the first place...

allsey87 commented 6 years ago

Also worth noting is a segfault which is produced for a similar reason as the QObject::disconnect: Unexpected null parameter warning. I am to produce this sometime when transitioning from having a light source selected and then selecting a robot (or vice versa). Attached is a backtrace that shows that this due to an oversight in the code where it is assumed that m_pcLuaVariableTree->model() is always a valid pointer. This assumption breaks when we consider some classes of entities such as those without controllers and perhaps in this case, entities which are not composable.

alanmillard commented 6 years ago

I'm able to reproduce the error with the following code, which does not include a light source: obstacle_avoidance.zip

alanmillard commented 6 years ago

Running ARGoS in gdb shows that the above code segfaults on line 773 of qtopengl_lua_main_window.cpp:

m_pcLuaVariableTree->setRootIndex(m_pcLuaVariableTree->model()->index(0, 0));

See backtrace.txt for further detail.

ilpincy commented 6 years ago

Working on it now. Thanks @allsey87 and @alanmillard for your input!

ilpincy commented 6 years ago

So, I looked at the code. I made a Buzz editor based on the Lua editor, and the Buzz editor now has new features with respect to the Lua one. Along with the new features, the Buzz editor also solves all of the issues mentioned here. I guess the best option for me is to backport the Buzz editor into the Lua one, so we would get the new features as well. It'll take a day or two - done by the weekend.

allsey87 commented 6 years ago

@ilpincy sounds good. Can you make a decision on whether to accept my proposal for #40 first though? It really should take no more than 10 minutes and I did put in two weeks of work to get to that point... It would really be appreciated.