Linux chupnp crash

[dizygotheca]

Thanks. A very welcome project.

Building aivctrl (master) with clang version 6.0.0-1ubuntu2 gives disturbing warnings:

QtUPnP/upnp/status.hpp:113 & 119: warning: all paths through this function will call itself 
QtUPnP/aivctrl/aivwidgets/networkprogress.hpp:64: warning: field 'm_invalidInc' is uninitialized when used here
QtUPnP/upnp/controlpoint.cpp:149: warning: variable 'iDeviceType' is incremented both in the loop header and in the loop body 

Also, chupnp crashes on startup. In CDeviceMap::extractServiceComponents, subDevice is a reference to a (QSharedData) device.subDevices. For some reason, subDevices then gets cleared whilst the subdevice is being processed (multithreading?). Meaning subDevice becomes a dangling pointer to garbage. Assigning subDevices, rather than using a reference, prevents the crash but I don't know if its the complete/right solution. QList<CDevice> subDevices = device.subDevices ();

[ptstream]

Hi, Many thanks for your remarks.

1. QtUPnP/upnp/status.hpp:113 & 119: warning: all paths through this function will call itself I saw the problem of QtUPnP/upnp/status.hpp. Gcc does not generate a message. I will fix it rapidly. Moreover, probably I never call these functions 👎 .

2. QtUPnP/aivctrl/aivwidgets/networkprogress.hpp:64: warning: field 'm_invalidInc' is uninitialized when used here Ok the code is not very good and subject at compiler interpretation. I will fix it rapidly.

3. QtUPnP/upnp/controlpoint.cpp:149: warning: variable 'iDeviceType' is incremented both in the loop header and in the loop body Your remark is true. Propably it is a problem of messages not really thread (I think). You can make this change without any problem but to fix it I need a little time to fully understand the process that leads to the crash. The major reason, I can not produce the problem. If you make these changes, the embedded devices may not be up to date properly.

Regards, Patrice

[dizygotheca]

Looked properly today. Same issue when using gcc 7.3.0. I'm using Qt 5.9

Here's a stack trace. "other" is garbage at this point, as is "subdevice" in frame 10

1   QUrl::QUrl(QUrl const&)                                                          0x7ffff62e12bb 
2   QtUPnP::SDeviceData::SDeviceData                        device.cpp           42  0x475982       
3   QSharedDataPointer<QtUPnP::SDeviceData>::clone          qshareddata.h        232 0x47aa50       
4   QSharedDataPointer<QtUPnP::SDeviceData>::detach_helper  qshareddata.h        238 0x47a9a9       
5   QSharedDataPointer<QtUPnP::SDeviceData>::detach         qshareddata.h        74  0x47a97f       
6   QSharedDataPointer<QtUPnP::SDeviceData>::operator->     qshareddata.h        77  0x479ac9       
7   QtUPnP::CDevice::subDevices                             device.cpp           600 0x4786c5       
8   QtUPnP::CDeviceMap::insertDevice                        devicemap.cpp        100 0x451608       
9   QtUPnP::CDeviceMap::extractServiceComponents            devicemap.cpp        125 0x451793       
10  QtUPnP::CDeviceMap::extractServiceComponents            devicemap.cpp        121 0x451759       
11  QtUPnP::CDeviceMap::extractDevicesFromNotify            devicemap.cpp        170 0x451b95       
12  QtUPnP::CControlPoint::extractDevicesFromNotify         controlpoint.cpp     328 0x445893       
13  QtUPnP::CControlPoint::newDevicesDetected               controlpoint.cpp     214 0x445565       

At frame 8 "device" is becoming invalid when it is inserted into the map. The QSharedDataPointer "m_d" is valid but SDeviceData becomes garbage. Frames 1-7 are operating on garbage.

The QSharedPointer copy constructor is doing the delete when it is invoked from CDevice copy constructor by the QMap insertion.

The problem occurs because my device & subdevice have the same uuid (Snipped summary)

device  @0x5555559fa110 QtUPnP::CDevice &
    m_d @0x5555559dc6f0 QSharedDataPointer<QtUPnP::SDeviceData>
        [QSharedData]   ref: 1  QSharedData
        m_deviceType    "urn:schemas-upnp-org:device:InternetGatewayDevice:1"   QString
        m_friendlyName  "ARRIS TG2492LG-85 Router"  QString
        m_subDevices    <1 items>   QList<QtUPnP::CDevice>
            [0] @0x7fffcc0095b0 QtUPnP::CDevice
                m_d @0x5555559d9b50 QSharedDataPointer<QtUPnP::SDeviceData>
                    [QSharedData]   ref: 1  QSharedData
                    m_deviceType    "urn:schemas-upnp-org:device:WANDevice:1"   QString
                    m_friendlyName  "WANDevice" QString
                    m_parentUUID    "uuid:31323032-3631-3800-0000-c005c252f3ed" QString
                    m_uuid  "uuid:31323032-3631-3800-0000-c005c252f3ed" QString
        m_uuid  "uuid:31323032-3631-3800-0000-c005c252f3ed" QString

The device is inserted into the map first, then immediately replaced by its subdevice, which deletes the previous map value - its parent device! Because it is a reference the device is getting destroyed and iterating over its subdevices becomes invalid. Something needs to detach somewhere.

Hopefully that gives you some insight.

[ptstream]

Args!!!!!!!!!!!! same uuid. 👎 👎 👎 👎 👎 I had never met the case or even thought it could exist. I have commit some changes a few minutes before your message. This commit solves the warnings but not this problem. I make something to fix it. Thanks for your good ananlyse.

[ptstream]

I fixed the problem of sub-device uuid equal at parent uuid (I hope). I can't really test it because my internet gateway device and sub-device have different uuids. I just simulated it. I wait your return to close, or not, the issue. Thanks in advance.

[dizygotheca]

Still crashes in the same place, I'm afraid.

This is a "SuperHub 3" router from "Virgin" ISP. Quite common in the UK. That doesn't mean it's good or compliant though...

It uses a tree of embedded devices: router - WANDevice - WANConnectionDevice The child (WANDevice) is handled properly. However the grandchild (WANConnectionDevice) isn't because its parentUUID is now "uuid...&0" so the (uuid == parentUUID) fails and it still replaces the parent device in the map.

I changed extractServiceComponents to:

    QString         parentUUID = device.uuid ();
    QString         rootUUID   = parentUUID.split('&')[0];
    ...
      if (uuid == rootUUID)
    ...

Now chupnp doesn't crash and shows the tree of router devices under "Others" :) They also appear correctly in the Services tab with no services. They appear as "WANDevice [:65535]". Not a problem though.

If that fix suits you then this issue is fixed. Many thanks.

[ptstream]

You are right. It is a very bad code. I commit a new version slightly different I hope more general. If you have some time to test it, and if all is Ok I could close the issue. Many thanks for your contribution.

[dizygotheca]

That works. Close it. Thanks.

My initial aim was to discover uPnP services. That's easy, thanks to QtUPnP. Now I'm considering implementing a Media Renderer, but I don't see a way to publish/advertise a device/service. Any hints ? Or do I misunderstand what's possible ?

[ptstream]

Thanks for your return. I will close the issue.

Create a UPnP A/V renderer is not a simple project. QtUPnP is not ready for this because only the control point part of UPnP is implemented. For example, if your see the old project "Developer Tools for UPnP technology" (https://software.intel.com/en-us/articles/intel-software-for-upnp-technology-technology-overview/) the code to generate a control point and a device is in different parts. Of course, some parts are common (create a http server, manage UDP communications, but the over are different:

• A control point read the device caracteristics (services, variables, eventing, ...). A renderer must provide theses.
• A Control point do not know anything about the file format to stream, the renderer must decode correctly the content of a server.
• An a lot of other things.

[dizygotheca]

Thanks for confirming my suspicion. Browsing the code made me query how much of a uPnP Renderer device it already implements/supports.

My project is a media player, so the 'business logic' is already there. We have some old/incomplete/broken uPnP functionality based on bespoke SSDP/HTTP that I'd like to replace completely with a Qt solution. I believe it implements parts of a Control Point (server discovery) and (intended to) support a basic Renderer device as well.

So I was curious if it made sense to re-use/extend parts of QtUPnP in the same way. How much of it is common ? is the question I need to answer.

More research needed. Another day...

Regards