AddInManager crashes on closeAddIn when used without GUI

Issue #174 resolved
thomas kipp created an issue

When an addIn is instantiated without the Itom GUI but with direct use of the addInManager in c++ code, the program crashes when calling closeAddIn. The Problem lies in line 1108 in addInManagerPirvate.cpp:

if (QApplication::instance())
{
  if (aib->getCallInitInNewThread())
  {
    ItomSharedSemaphoreLocker moveToThreadLocker(new ItomSharedSemaphore());
    // this a (temporary?) workaround for application hanging on closing, using addInManager dll
    Qt::ConnectionType conType = (QApplication::instance() != NULL && !(m_pQCoreApp && QApplication::hasPendingEvents())) ? Qt::AutoConnection : Qt::DirectConnection;
    if (QMetaObject::invokeMethod(addIn, "moveBackToApplicationThread", conType, Q_ARG(ItomSharedSemaphore*, moveToThreadLocker.getSemaphore())))
    ...

When conType evaluates to Qt::DirectConnection, the addIn cannot be moved back to the application thread since the method has to be called from within the addIns thread.

The line with the conType seems to be copy and paste from line 1091.

Comments (3)

  1. M. Gronle

    Hi Thomas,

    can you check if the unittest_addinmanager runs for you? This unittest contains one test, that externally loads the AddInManager DLL and opens a DummyMotor instance. Afterwards this instance is closed and the AddInManager singleton is closed, too. In order to run the C++ unittests for itom, you have to enable them in CMake. If you use Visual Studio I recommend the Google Test Adapter Extension. Then you can also start the unittests from the test exporer toolbar in VS:

    I just updated the unittest, such that the opened DummyMotor instance is closed again to test your issue. For me it works. However, the conType is AutoConnection,
    since probably no pending events are available.

    Now, there are two things:

    1. We need to change these lines in any cases, since hasPendingEvents is deprecated in Qt and has no replacement, since Qt does not offer this in future versions.
    2. I personally do not understand why it is necessary to have this switch for the connection type. A colleague of you delevoped this part of the code to support the standalone AddInManager. I guess nobody else is using this kind of standalone AddInManager. What would happen if the connection is always AutoConnection?
      Could you please try to find a fix for this without using hasPendingEvents? Or do you have an idea why it would not be possible to use AutoConnection in any cases?

    Thanks.

    Marc

  2. thomas kipp reporter

    Hi Marc,

    the unittest runs fine. The problem only occurs with pending events.

    Eliminating the conType and using autoConnection on "moveBackToApplicationThread" and also in the call to "close" above solved the problem as expected. Our software runs fine without the conType so I think it can be removed. Will you implement this or should I make a pull request.

    Thank you for the fast answer.

    Thomas

  3. Log in to comment