Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Program hangs with Aws::Iot::MqttClient:NewConnection() #556

Closed
nabelekt opened this issue Oct 19, 2023 · 4 comments
Closed

Program hangs with Aws::Iot::MqttClient:NewConnection() #556

nabelekt opened this issue Oct 19, 2023 · 4 comments
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@nabelekt
Copy link

Describe the bug

It is likely that this is user error rather than a bug. If so, please educate me on what I am doing wrong here.

Here's my simplified code:

// Some code adopted from https://github.com/aws/aws-iot-device-sdk-cpp-v2/blob/main/samples/pub_sub/basic_pub_sub/main.cpp

#include <aws/crt/Api.h>
#include <aws/iot/MqttClient.h>
#include <iostream>

#define DEBUG_PRINT std::cout << __FILE__ << ":" << __LINE__ << "\n" << std::flush;

class AwsIotMqttHandler
{
    public:

        typedef struct
        {
            Aws::Crt::String certificateFilepath;
            Aws::Crt::String privateKeyFilepath;
            Aws::Crt::String rootCertificateFilepath;
            Aws::Crt::String endpoint;
        } connectionParameters_t;

        AwsIotMqttHandler();

        void InitializeConnection(connectionParameters_t connectionParameters);

    protected:

        std::shared_ptr<Aws::Crt::Mqtt::MqttConnection> m_connection;
};

AwsIotMqttHandler::AwsIotMqttHandler()
{
}

void AwsIotMqttHandler::InitializeConnection(connectionParameters_t connectionParameters)
{
    // Do some global initialization for the API
    Aws::Crt::ApiHandle apiHandle;

    // Create the MQTT builder and populate it with connection parameters
    auto clientConfigBuilder =
        Aws::Iot::MqttClientConnectionConfigBuilder(connectionParameters.certificateFilepath.c_str(),
                                                    connectionParameters.privateKeyFilepath.c_str());
    clientConfigBuilder.WithEndpoint(connectionParameters.endpoint);
    clientConfigBuilder.WithCertificateAuthority(connectionParameters.rootCertificateFilepath.c_str());
    DEBUG_PRINT

    // Create the MQTT connection from the MQTT builder
    Aws::Iot::MqttClientConnectionConfig clientConfig = clientConfigBuilder.Build();
    if (!clientConfig)
    {
        std::cerr << std::string("Client configuration initialization failed with error: ")
                        + std::string(Aws::Crt::ErrorDebugString(clientConfig.LastError()));
        return;
    }
    DEBUG_PRINT
    Aws::Iot::MqttClient client = Aws::Iot::MqttClient();
    m_connection = client.NewConnection(clientConfig);
    DEBUG_PRINT
    if (m_connection != nullptr && !*m_connection)
    {
        std::cerr << std::string("MQTT connection creation failed with error: ")
                        + std::string(Aws::Crt::ErrorDebugString(m_connection->LastError()));
        return;
    }
    DEBUG_PRINT
}

int main(int argc, char** argv)
{
    std::cout << "\nStarting: " << argv[0] << "\n";

    DEBUG_PRINT

    AwsIotMqttHandler::connectionParameters_t connectionParameters;
    connectionParameters.certificateFilepath     = "/root/gateway/aws_keys/certificate.pem.crt";
    connectionParameters.privateKeyFilepath      = "/root/gateway/aws_keys/private.pem.key";
    connectionParameters.rootCertificateFilepath = "/root/gateway/aws_keys/AmazonRootCA1.pem";
    connectionParameters.endpoint                = "a1ivqb4fk63pz7-ats.iot.us-east-1.amazonaws.com";

    AwsIotMqttHandler awsIotMqttHandler;
    awsIotMqttHandler.InitializeConnection(connectionParameters);

    DEBUG_PRINT

    return 0;
}

Expected Behavior

Print


Starting: ./aws_mqtt_connection_hang.exe
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:72
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:45
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:55
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:58
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:65
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:83

and exit. This is what happens if I comment out the m_connection = client.NewConnection(clientConfig); line.

Current Behavior

Prints


Starting: ./aws_mqtt_connection_hang.exe
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:72
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:45
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:55
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:58
/root/gateway/experiments/aws_mqtt_connection_hang.cpp:65

(no /root/gateway/experiments/aws_mqtt_connection_hang.cpp:83 line) and and does not exit. Three threads are left running:
aws_mqtt_connec
AwsEventLoop 1
AwsEventLoop 2

Reproduction Steps

  1. Change the filepaths and endpoint in the main function.
  2. Build the code under g++-13 c++20.
  3. Run the code.

Possible Solution

No response

Additional Information/Context

No response

aws-crt-cpp version used

AWS CRT C++ 0.24.3

Compiler and version used

g++-13 (Ubuntu 13.1.0-8ubuntu1~22.04) 13.1.0

Operating System and version

Ubuntu 22.04.2 LTS

@nabelekt nabelekt added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 19, 2023
@nabelekt
Copy link
Author

nabelekt commented Oct 20, 2023

I think the issue has to do with the scope of m_connection. If I change

m_connection = client.NewConnection(clientConfig);

to

auto connection = client.NewConnection(clientConfig);

the problem goes away. Still not sure why the created connection being in scope prevents the program from continuing. And this doesn't help me much for creating a class that allows creating the connection, disconnecting, and using the connection to be handled in separate methods.

@xiazhvera
Copy link
Contributor

Looking into the issue, the program stalls on the destructor of apiHandle, and apiHandle was waiting on destroy of the m_connection.
You should keep the apiHandle while you are using the API. As the m_connection is still alive, you have to keep the apiHandle.

You can initialize the ApiHandle in main function and remove it from the member function. Example:

int main(int argc, char** argv)
{
    // Init ApiHandle here, and remove it from `InitializeConnection `
    Aws::Crt::ApiHandle apiHandle;
    
   std::cout << "\nStarting: " << argv[0] << "\n";

    DEBUG_PRINT

    AwsIotMqttHandler::connectionParameters_t connectionParameters;
    connectionParameters.certificateFilepath     = "/root/gateway/aws_keys/certificate.pem.crt";
    connectionParameters.privateKeyFilepath      = "/root/gateway/aws_keys/private.pem.key";
    connectionParameters.rootCertificateFilepath = "/root/gateway/aws_keys/AmazonRootCA1.pem";
    connectionParameters.endpoint                = "a1ivqb4fk63pz7-ats.iot.us-east-1.amazonaws.com";

    AwsIotMqttHandler awsIotMqttHandler;
    awsIotMqttHandler.InitializeConnection(connectionParameters);

    DEBUG_PRINT

    return 0;
}

@xiazhvera
Copy link
Contributor

xiazhvera commented Oct 20, 2023

Also, that is the reason why the issue was resolved by changing the connection to a local variable.

auto connection = client.NewConnection(clientConfig);

As now the connection is a local variable, it will be destroyed when it goes out of the scope as the apiHandle does.

@nabelekt
Copy link
Author

Solved! Thank you, @xiazhvera!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants