Fatal error condition... channel_bootstrap.c:743: bootstrap Debugging and Documentation Question

[pwolfe1]

Hello,

I have a question about a specific issue I don't understand that connects to a more general debugging question.

I have written a C++ class IpcClient with a constructor and a connect method as follows:

    IpcClient::IpcClient() {

        log::info("Building IpcClient...");

        apiHandle = std::make_unique<ApiHandle>(g_allocator);

        Io::EventLoopGroup eventLoopGroup(1);
        Io::DefaultHostResolver socketResolver(eventLoopGroup, 64, 30);
        Io::ClientBootstrap bootstrap(eventLoopGroup, socketResolver);
        ggcIpcClient = std::make_unique<GreengrassCoreIpcClient>(bootstrap);

        log::info("Built IpcClient!");
    }

    bool IpcClient::connect() {
    
        IpcClientLifecycleHandler ipcLifecycleHandler;
        auto connectionStatus = ggcIpcClient->Connect(ipcLifecycleHandler).get();
        if (!connectionStatus) {
            log::error("Failed to establish IPC connection: {}", connectionStatus.StatusToString());
            return false;
        }
    
        return true;

    }

When run, this code hits a fatal error in the connect method:

2023-04-18T15:36:19.497Z [WARN] (Copier) com.example.MyGreengrassApp: stderr. Fatal error condition occurred in /usr/src/debug/aws-c-io/0.13.19-r0/git/source/channel_bootstrap.c:743: bootstrap.

I suspected this was related to the scope or lifecycle of the Io::ClientBootstrap based on the error message. By moving the connect body to the constructor bootstrap is local to, I was able to resolve the error.

Working version:

    IpcClient::IpcClient() {

        log::info("Building IpcClient...");

        apiHandle = std::make_unique<ApiHandle>(g_allocator);

        Io::EventLoopGroup eventLoopGroup(1);
        Io::DefaultHostResolver socketResolver(eventLoopGroup, 64, 30);
        Io::ClientBootstrap bootstrap(eventLoopGroup, socketResolver);
        ggcIpcClient = std::make_unique<GreengrassCoreIpcClient>(bootstrap);

        log::info("Built IpcClient!");
        
        IpcClientLifecycleHandler ipcLifecycleHandler;
        auto connectionStatus = ggcIpcClient->Connect(ipcLifecycleHandler).get();
        if (!connectionStatus) {
            log::error("Failed to establish IPC connection: {}", connectionStatus.StatusToString());
            return;
        }

    }

Similarly, I could use a default constructor and rename connect to init and moving this block of code into it. What I am confused about is where I can find information such as that the Io::ClientBootstrap needs to be local to the block of code Connect is called in, even though bootstrap is passed to the GreengrassCoreIpcClient constructor - or a clarification that that is not actually the source of this error.

I have found the documentation in the Developer Guide great for understanding general Greengrass concepts and getting simple C++ programs up and running by replicating the examples. But is there a better source for detailed explanations on how to use the C++ API and its nuances? I don't see anything about Io::ClientBootstrap in the Connect docs

[bretambrose]

It's not quite that things need to be local, it's that if you're building something and passing in a reference to an object, then that object needs to outlive what you're building. The samples tend to put everything in a quasi-global (main) scope for simplicity. But unfortunately, given a C++ environment, you need to always be thinking about object lifetimes.

Personally, I wish we'd never allowed by-value construction of complex objects (bootstrap, event loop group, etc..) and only allowed shared_ptr-returning-factory construction and everything configured by shared pointers not direct references. I think the semantics would have been much clearer and safer to use, but unfortunately, that's not the API we have.

[pwolfe1]

Interesting - that does make sense and I think I understand this situation a bit better now.

if you're building something and passing in a reference to an object, then that object needs to outlive what you're building.

So it isn't that those need to live as long as it takes to call Connect, but that they should live as long as the GreengrassCoreIpcClient whose constructor their references are passed to. In that case, what needs to outlive ggcIpcClient? Would that be all of eventLoopGroup, socketResolver, and bootstrap? It's really not an issue to just have those be additional member variables which would mean they all would have the same lifetime as they are all within IpcClient, but I want to make sure I am properly understanding the underlying concept.

[pwolfe1]

Personally, I wish we'd never allowed by-value construction of complex objects (bootstrap, event loop group, etc..) and only allowed shared_ptr-returning-factory construction and everything configured by shared pointers not direct references

This is what I was expecting although I now see it's clearly not the case for the constructor or Connect - based on the documentation it seems like the rest of the GreengrassCoreIpcClient API is implemented this way though which is great.

[bretambrose]

So it isn't that those need to live as long as it takes to call Connect, but that they should live as long as the GreengrassCoreIpcClient whose constructor their references are passed to. In that case, what needs to outlive ggcIpcClient? Would that be all of eventLoopGroup, socketResolver, and bootstrap? It's really not an issue to just have those be additional member variables which would mean they all would have the same lifetime as they are all within IpcClient, but I want to make sure I am properly understanding the underlying concept.

Those three (elg, client bootstrap, and host resolver) should be sufficient. I am wondering though if we could switch the semantics of those objects interally. They're trivial wrappers around ref-counted C objects, so if we made them copyable with appropriate ref-counting work, then the client could just copy them and their lifetimes would no longer be your problem.

This is not something we could do universally though. Any object (like network clients) with bound callbacks would have to stay uncopyable.

[pwolfe1]

They're trivial wrappers around ref-counted C objects, so if we made them copyable with appropriate ref-counting work, then the client could just copy them and their lifetimes would no longer be your problem.

Yeah, that makes sense. I would be happy to take a look into this when I have a chance.

[github-actions[bot]]

Hello! Reopening this discussion to make it searchable.