{"action":"reopened","issue":{"url":"https://api.github.com/repos/mavlink/qgroundcontrol/issues/4343","repository_url":"https://api.github.com/repos/mavlink/qgroundcontrol","labels_url":"https://api.github.com/repos/mavlink/qgroundcontrol/issues/4343/labels{/name}","comments_url":"https://api.github.com/repos/mavlink/qgroundcontrol/issues/4343/comments","events_url":"https://api.github.com/repos/mavlink/qgroundcontrol/issues/4343/events","html_url":"https://github.com/mavlink/qgroundcontrol/issues/4343","id":197409724,"number":4343,"title":"QGC crashes when sending Vehicle MAVLink message on a link! (at unexpected times)","user":{"login":"samacumen","id":1699546,"avatar_url":"https://avatars.githubusercontent.com/u/1699546?v=3","gravatar_id":"","url":"https://api.github.com/users/samacumen","html_url":"https://github.com/samacumen","followers_url":"https://api.github.com/users/samacumen/followers","following_url":"https://api.github.com/users/samacumen/following{/other_user}","gists_url":"https://api.github.com/users/samacumen/gists{/gist_id}","starred_url":"https://api.github.com/users/samacumen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/samacumen/subscriptions","organizations_url":"https://api.github.com/users/samacumen/orgs","repos_url":"https://api.github.com/users/samacumen/repos","events_url":"https://api.github.com/users/samacumen/events{/privacy}","received_events_url":"https://api.github.com/users/samacumen/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"dogmaphobic","id":749243,"avatar_url":"https://avatars.githubusercontent.com/u/749243?v=3","gravatar_id":"","url":"https://api.github.com/users/dogmaphobic","html_url":"https://github.com/dogmaphobic","followers_url":"https://api.github.com/users/dogmaphobic/followers","following_url":"https://api.github.com/users/dogmaphobic/following{/other_user}","gists_url":"https://api.github.com/users/dogmaphobic/gists{/gist_id}","starred_url":"https://api.github.com/users/dogmaphobic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dogmaphobic/subscriptions","organizations_url":"https://api.github.com/users/dogmaphobic/orgs","repos_url":"https://api.github.com/users/dogmaphobic/repos","events_url":"https://api.github.com/users/dogmaphobic/events{/privacy}","received_events_url":"https://api.github.com/users/dogmaphobic/received_events","type":"User","site_admin":false},"assignees":[{"login":"dogmaphobic","id":749243,"avatar_url":"https://avatars.githubusercontent.com/u/749243?v=3","gravatar_id":"","url":"https://api.github.com/users/dogmaphobic","html_url":"https://github.com/dogmaphobic","followers_url":"https://api.github.com/users/dogmaphobic/followers","following_url":"https://api.github.com/users/dogmaphobic/following{/other_user}","gists_url":"https://api.github.com/users/dogmaphobic/gists{/gist_id}","starred_url":"https://api.github.com/users/dogmaphobic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dogmaphobic/subscriptions","organizations_url":"https://api.github.com/users/dogmaphobic/orgs","repos_url":"https://api.github.com/users/dogmaphobic/repos","events_url":"https://api.github.com/users/dogmaphobic/events{/privacy}","received_events_url":"https://api.github.com/users/dogmaphobic/received_events","type":"User","site_admin":false},{"login":"DonLakeFlyer","id":5876851,"avatar_url":"https://avatars.githubusercontent.com/u/5876851?v=3","gravatar_id":"","url":"https://api.github.com/users/DonLakeFlyer","html_url":"https://github.com/DonLakeFlyer","followers_url":"https://api.github.com/users/DonLakeFlyer/followers","following_url":"https://api.github.com/users/DonLakeFlyer/following{/other_user}","gists_url":"https://api.github.com/users/DonLakeFlyer/gists{/gist_id}","starred_url":"https://api.github.com/users/DonLakeFlyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DonLakeFlyer/subscriptions","organizations_url":"https://api.github.com/users/DonLakeFlyer/orgs","repos_url":"https://api.github.com/users/DonLakeFlyer/repos","events_url":"https://api.github.com/users/DonLakeFlyer/events{/privacy}","received_events_url":"https://api.github.com/users/DonLakeFlyer/received_events","type":"User","site_admin":false}],"milestone":null,"comments":5,"created_at":"2016-12-23T18:16:54Z","updated_at":"2016-12-23T20:07:40Z","closed_at":null,"body":"I would like to bring to your notice that QGC crashes at unexpected times when using Serial/or multiple links. I recently figured out why and where this happens and the root cause. The culprit is the way we have written the function priorityLink() and its usage while sending mavlink messages.\r\n\r\nIf AutoConnect UDP is used, there will be at least one link in the **Vehicle::_links** Qlist. When the serial USB is connected, there is this serial link. What this logic of priorityLink() does is it gives Serial the priority and if it is not found, the first link in the _links list is returned.\r\n\r\n```\r\n/// @return Direct usb connection link to board if one, NULL if none\r\nLinkInterface* Vehicle::priorityLink(void)\r\n{\r\n    foreach (LinkInterface* link, _links) {\r\n        if (link->isConnected()) {\r\n            SerialLink* pSerialLink = qobject_cast<SerialLink*>(link);\r\n\t    if (pSerialLink) {\r\n                return link;\r\n            }\r\n        }\r\n    }\r\n\r\n    return _links.count() ? _links[0] : NULL;\r\n}\r\n```\r\n\r\n**Two problems:**\r\n\r\n1) return _links.count() ? _links[0] : NULL; \r\nWhen no vehicle is connected, we disable the QGC GUI - so we are safe. But during certain heavy processing, like sending a big mission to the vehicle, if the link (serial/other) is disconnected - the above could/is certainly NULL. Consequences? Crash of QGC, as we use prioritylink()->mavlinkChannel() while sending each message. Access on NULL instance!\r\n\r\n2) If there are more than 1 links (apart from serial, say a UDP and a TCP i.e. Serial, UDP, TCP in the list) for the same vehicle (which is allowed) - which link must be chosen if Serial is not available? The first in the list after Serial, i.e. UDP -> does not make much sense. This indicates the TCP link will never be used.\r\n\r\nSomehow it feels to me that the implementation of PriorityLink() is incomplete. Suggest to at least place a NULL check apart from link not being connected while sending ANY Mavlink message to the vehicle. \r\n\r\nPS: Also, notice that isConnected() for UDP link will never be 'false' (unless user disconnects manually) and isConnected() for Serial will usually not occur (as the link object is lost immediately on a disconnect).\r\n\r\n@LorenzMeier , @DonLakeFlyer "}}
{"actor":{"display_login":"samacumen"}}