mavlink / MAVSDK

API and library for MAVLink compatible systems written in C++17
https://mavsdk.mavlink.io
BSD 3-Clause "New" or "Revised" License
635 stars 509 forks source link

Consecutive param sets trigger exception #1263

Closed julianoes closed 3 years ago

julianoes commented 3 years ago

@dakejahl reports:

This causes an exception

_param_interface->set_param_int(param_name, val);
_param_interface->set_param_int(param_name, val);

terminate called after throwing an instance of 'std::future_error'
what():  std::future_error: Promise already satisfied

Is there a reason why setting parameters back to back like this would be a problem? If I add a sleep in between it works

julianoes commented 3 years ago

I tried reproducing this against SITL and I don't see it happen with this snippet:

        const Param::Result set_result3 = param->set_param_int("NAV_RCL_ACT", 0);
        EXPECT_EQ(set_result3, Param::Result::Success);
        const Param::Result set_result4 = param->set_param_int("NAV_RCL_ACT", 1);
        EXPECT_EQ(set_result4, Param::Result::Success);

I've added that to https://github.com/mavlink/MAVSDK/blob/develop/src/integration_tests/param.cpp.

@dakejahl could you provide me with a minimal example to reproduce this?

julianoes commented 3 years ago

@dakejahl I can't see where it goes wrong in code either. Please generate a backtrace and paste it here.

JonasVautherin commented 3 years ago

Quick guess: this line is called twice.

julianoes commented 3 years ago

@JonasVautherin right, that's also my guess but I don't understand how/why yet.

dakejahl commented 3 years ago

Haven't had time to dig into this, I'll report back soon

dakejahl commented 3 years ago

I'm not sure how to generate a backtrace, but here's a minimal example to reproduce. This is being tested on a raspberry pi Zero W using MAVSDK 0.31.0. I tried installing a SIGSEV handler but couldn't figure it out, this example will cause the exception though.

#include <stdio.h>
#include <execinfo.h>
#include <signal.h>
#include <stdlib.h>
#include <unistd.h>

#include <cstdint>
#include <chrono>
#include <iostream>
#include <thread>
#include <memory>

#include <mavsdk/mavsdk.h>
#include <mavsdk/plugins/telemetry/telemetry.h>
#include <mavsdk/plugins/param/param.h>

using namespace mavsdk;
using namespace std::this_thread;
using namespace std::chrono;

#define ERROR_CONSOLE_TEXT "\033[31m" // Turn text on console red
#define TELEMETRY_CONSOLE_TEXT "\033[34m" // Turn text on console blue
#define NORMAL_CONSOLE_TEXT "\033[0m" // Restore normal console colour

// Globals
Mavsdk* _mavsdk;
System* _system;

int setup_mavsdk_connection()
{
    // MAVSDK SETUP
    _mavsdk = new Mavsdk();

    std::string connection_url = "serial:///dev/serial0:57600";
    ConnectionResult connection_result = _mavsdk->add_any_connection(connection_url);
    bool discovered_system = false;

    if (connection_result != ConnectionResult::Success) {
        std::cout << ERROR_CONSOLE_TEXT << "Connection failed: " << connection_result
                  << NORMAL_CONSOLE_TEXT << std::endl;
        return 0;
    }

        _system = &_mavsdk->system();

    std::cout << "Waiting to discover system..." << std::endl;
    _mavsdk->register_on_discover([&discovered_system](uint64_t uuid) {
        std::cout << "Discovered system with UUID: " << uuid << std::endl;
        discovered_system = true;
    });

    // We usually receive heartbeats at 1Hz
    sleep_for(seconds(2));

    if (!discovered_system) {
        std::cout << ERROR_CONSOLE_TEXT << "No system found, exiting." << NORMAL_CONSOLE_TEXT
                  << std::endl;
        return 0;
    }

    return 1;
}

int main(int argc, char **argv)
{
    (void)argc;
    (void)argv;

    if (!setup_mavsdk_connection()) {
        std::cout << "Failed to connect to flight controller, exiting." << ERROR_CONSOLE_TEXT << std::endl;
        return 0;
    }

    auto telemetry = std::make_shared<Telemetry>(*_system);

    // Check if vehicle is ready to arm
    if (telemetry->health_all_ok() != true) {
        std::cout << "Vehicle is getting ready" << std::endl;
        sleep_for(seconds(1));
    }

    auto param = std::make_shared<Param>(*_system);

    // APM sends/receives all parameters as floats
    param->set_param_float("FRAME_CLASS", 3);
    param->set_param_float("FRAME_CLASS", 4);

    std::cout << "Exiting successfully" << std::endl;
}
dakejahl commented 3 years ago

apologies for the pointers.. but TBH I hate using std::make_shared and all that other excessive syntax. Plus references everywhere can be pretty inconvenient

julianoes commented 3 years ago

@dakejahl to get a backtrace, just run it with gdb.

Or save a core dump: https://stackoverflow.com/questions/17965/how-to-generate-a-core-dump-in-linux-on-a-segmentation-fault

julianoes commented 3 years ago

TBH I hate using std::make_shared and all that other excessive syntax.

std::shared_ptr for plugins is no longer necessary, and we're updating the examples: https://github.com/mavlink/MAVSDK/pull/1270/files#diff-76a888fbf298963b3e21d48a9d7efd522e32ac8e65446b29311addaf78602453R104-R106

all that other excessive syntax.

I would challenge that.

Plus references everywhere can be pretty inconvenient

It's often inconvenience against safety, I'd say.

julianoes commented 3 years ago

@dakejahl I still can't reproduce it, just tried against against SITL.

dakejahl commented 3 years ago

Here's a backtrace when I run it with gdb

(gdb) run
Starting program: /home/pi/prism_os/tests/param_set_example 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
[09:59:28|Info ] MAVSDK version: v0.31.0-17-g00afc8a9-dirty (mavsdk_impl.cpp:27)
[New Thread 0xb6c20450 (LWP 4336)]
[New Thread 0xb641f450 (LWP 4337)]
[New Thread 0xb5c1e450 (LWP 4338)]
[09:59:28|Debug] New: System ID: 0 Comp ID: 0 (mavsdk_impl.cpp:405)
[New Thread 0xb541d450 (LWP 4339)]
Waiting to discover system...
[09:59:28|Debug] Component Autopilot (1) added. (system_impl.cpp:340)
[09:59:28|Debug] Discovered 1 component(s) (UUID: 1) (system_impl.cpp:513)
Discovered system with UUID: 1
Vehicle is getting ready
terminate called after throwing an instance of 'std::future_error'
  what():  std::future_error: Promise already satisfied

Thread 4 "param_set_examp" received signal SIGABRT, Aborted.
[Switching to Thread 0xb5c1e450 (LWP 4338)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) backtrace
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0xb6c3a230 in __GI_abort () at abort.c:79
#2  0xb6e948d8 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#3  0xb6e925b0 in ?? () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#4  0xb6e92624 in std::terminate() () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#5  0xb6e92990 in __cxa_throw () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#6  0xb6ebbc84 in std::__throw_future_error(int) () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#7  0x000423e0 in std::__future_base::_State_base::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) (this=0xc4500, __res=..., __ignore_failure=false) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/future:360
#8  0x00076ec0 in std::promise<mavsdk::MAVLinkParameters::Result>::set_value (this=0xbefff474, __r=@0xb5c1cff0: mavsdk::MAVLinkParameters::Result::Success)
    at /usr/arm-linux-gnueabihf/include/c++/4.8.3/future:987
#9  0x0007017c in mavsdk::MAVLinkParameters::__lambda4::operator() (__closure=0xc3f80, result=mavsdk::MAVLinkParameters::Result::Success)
    at /work/src/core/mavlink_parameters.cpp:70
#10 0x00072de8 in std::_Function_handler<void(mavsdk::MAVLinkParameters::Result), mavsdk::MAVLinkParameters::set_param(const string&, const mavsdk::MAVLinkParameters::ParamValue&, bool)::__lambda4>::_M_invoke(const std::_Any_data &, mavsdk::MAVLinkParameters::Result) (__functor=..., 
    __args#0=mavsdk::MAVLinkParameters::Result::Success) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:2071
#11 0x00076b44 in std::function<void (mavsdk::MAVLinkParameters::Result)>::operator()(mavsdk::MAVLinkParameters::Result) const (this=0xc48c4, 
    __args#0=mavsdk::MAVLinkParameters::Result::Success) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:2464
#12 0x00071490 in mavsdk::MAVLinkParameters::process_param_value (this=0xc258c, message=...) at /work/src/core/mavlink_parameters.cpp:333
#13 0x0007d244 in std::_Mem_fn<void (mavsdk::MAVLinkParameters::*)(__mavlink_message const&)>::operator()<__mavlink_message const&, void> (this=0xc42c8, 
    __object=0xc258c) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:601
#14 0x0007be44 in std::_Bind<std::_Mem_fn<void (mavsdk::MAVLinkParameters::*)(__mavlink_message const&)> (mavsdk::MAVLinkParameters*, std::_Placeholder<1>)>::__call<void, __mavlink_message const&, 0u, 1u>(std::tuple<__mavlink_message const&>&&, std::_Index_tuple<0u, 1u>) (this=0xc42c8, __args=...)
    at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1296
#15 0x0007a74c in std::_Bind<std::_Mem_fn<void (mavsdk::MAVLinkParameters::*)(__mavlink_message const&)> (mavsdk::MAVLinkParameters*, std::_Placeholder<1>)>::operator()<__mavlink_message const&, void>(__mavlink_message const&) (this=0xc42c8) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1355
#16 0x000786a0 in std::_Function_handler<void (__mavlink_message const&), std::_Bind<std::_Mem_fn<void (mavsdk::MAVLinkParameters::*)(__mavlink_message const&)> (mavsdk::MAVLinkParameters*, std::_Placeholder<1>)> >::_M_invoke(std::_Any_data const&, __mavlink_message const&) (__functor=..., __args#0=...)
--Type <RET> for more, q to quit, c to continue without paging--c
   rm-linux-gnueabihf/include/c++/4.8.3/functional:2071
#17 0x00082714 in std::function<void (__mavlink_message const&)>::operator()(__mavlink_message const&) const (this=0xc3fb4, __args#0=...) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:2464
#18 0x000823d4 in mavsdk::MAVLinkMessageHandler::process_message (this=0xc2508, message=...) at /work/src/core/mavlink_message_handler.cpp:55
#19 0x0003ae3c in mavsdk::SystemImpl::process_mavlink_message (this=0xc24a0, message=...) at /work/src/core/system_impl.cpp:128
#20 0x0001542c in mavsdk::MavsdkImpl::receive_message (this=0xc1068, message=...) at /work/src/core/mavsdk_impl.cpp:137
#21 0x00025dec in std::_Mem_fn<void (mavsdk::MavsdkImpl::*)(__mavlink_message&)>::operator()<__mavlink_message&, void> (this=0xc1ff0, __object=0xc1068) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:601
#22 0x00025c98 in std::_Bind<std::_Mem_fn<void (mavsdk::MavsdkImpl::*)(__mavlink_message&)> (mavsdk::MavsdkImpl*, std::_Placeholder<1>)>::__call<void, __mavlink_message&, 0u, 1u>(std::tuple<__mavlink_message&>&&, std::_Index_tuple<0u, 1u>) (this=0xc1ff0, __args=...) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1296
#23 0x00025954 in std::_Bind<std::_Mem_fn<void (mavsdk::MavsdkImpl::*)(__mavlink_message&)> (mavsdk::MavsdkImpl*, std::_Placeholder<1>)>::operator()<__mavlink_message&, void>(__mavlink_message&) (this=0xc1ff0) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1355
#24 0x000256f4 in std::_Function_handler<void (__mavlink_message&), std::_Bind<std::_Mem_fn<void (mavsdk::MavsdkImpl::*)(__mavlink_message&)> (mavsdk::MavsdkImpl*, std::_Placeholder<1>)> >::_M_invoke(std::_Any_data const&, __mavlink_message&) (__functor=..., __args#0=...) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:2071
#25 0x00037624 in std::function<void (__mavlink_message&)>::operator()(__mavlink_message&) const (this=0xc1f84, __args#0=...) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:2464
#26 0x00037190 in mavsdk::Connection::receive_message (this=0xc1f80, message=...) at /work/src/core/connection.cpp:43
#27 0x0002a12c in mavsdk::SerialConnection::receive (this=0xc1f80) at /work/src/core/serial_connection.cpp:321
#28 0x0002be30 in std::_Mem_fn<void (mavsdk::SerialConnection::*)()>::operator()<, void>(mavsdk::SerialConnection*) const (this=0xc2188, __object=0xc1f80) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:601
#29 0x0002bd5c in std::_Bind_simple<std::_Mem_fn<void (mavsdk::SerialConnection::*)()> (mavsdk::SerialConnection*)>::_M_invoke<0u>(std::_Index_tuple<0u>) (this=0xc2184) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1732
#30 0x0002bc00 in std::_Bind_simple<std::_Mem_fn<void (mavsdk::SerialConnection::*)()> (mavsdk::SerialConnection*)>::operator()() (this=0xc2184) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/functional:1720
#31 0x0002bb8c in std::thread::_Impl<std::_Bind_simple<std::_Mem_fn<void (mavsdk::SerialConnection::*)()> (mavsdk::SerialConnection*)> >::_M_run() (this=0xc2178) at /usr/arm-linux-gnueabihf/include/c++/4.8.3/thread:115
#32 0xb6ebdce8 in ?? () from /usr/lib/arm-linux-gnueabihf/libstdc++.so.6
#33 0xb6f6d494 in start_thread (arg=0xb5c1e450) at pthread_create.c:486
#34 0xb6cfa578 in ?? () at ../sysdeps/unix/sysv/linux/arm/clone.S:73 from /lib/arm-linux-gnueabihf/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
julianoes commented 3 years ago

This is what I had expected but I still can't understand how it could happen by looking at the code. And I could not trigger anything with sanitizers, valgrind, sleeps, double param sendings, etc..

@dakejahl could you share any changes you have made on top of v0.31.0?

julianoes commented 3 years ago

@dakejahl is this still an issue? Did you ever encounter it again? I still have not been able to replicate it.

dakejahl commented 3 years ago

I haven't dug into it anymore. My changes should not affect this code path in particular. I suspect it's probably an architecture specific issue (Zero W is ARMv6 and the compiler is arm-linux-gnueabihf-g++)

dakejahl commented 3 years ago

If you have a Zero W on hand you can try that example I posted above, here's my docker file too for reference

Dockerfile

FROM dockcross/linux-armv6

ENV DEFAULT_DOCKCROSS_IMAGE my_cool_image

RUN wget http://sourceforge.net/projects/boost/files/boost/1.58.0/boost_1_58_0.tar.bz2/download && \
    mv download boost.tar.bz2 && \
    tar xjvf boost.tar.bz2 && \
    cd boost_1_58_0/ && \
    echo "using gcc : arm : arm-linux-gnueabihf-g++ ;" >> project-config.jam && \
    ./bootstrap.sh --show-libraries && \
    sudo ./bjam --with-filesystem link=static install -j10 && \
    cd /usr/local/lib && sudo rm libcurl.* && \
    cd /usr/local/include/ && sudo rm -rf curl/

You already know this but to use it...

docker build -t my_cool_image .
docker run my_cool_image > raspberry-armv6-container
chmod +x raspberry-armv6-container
sudo mv raspberry-armv6-container /usr/bin
julianoes commented 3 years ago

Aha, that's a good point about RPi Zero. I can give that a try. Thx for the note.

julianoes commented 3 years ago

Just to follow up. I did try this on a RPi Zero against a board with ArduPilot flashed, and I could not reproduce it.

julianoes commented 3 years ago

Closing, please re-open if it still happens and if you can give me instructions how to reproduce it, thanks.