Tradias/asio-grpc

Why I get const correctness problems

Closed this issue · 16 comments

Hi again, sorry for having to ask again but the time I have to incorporate this in our project is close to being exhaused and my previous experience with asio is not proving helpful here.

I have copied the piece of code below from the generic server into our code and I am getting

error: passing ‘const agrpc::b::GrpcContext’ as ‘this’ argument discards qualifiers [-fpermissive]

errors, regardless how I add or remove const in the functions. Here is the code

struct GenericRequestHandler {
	using executor_type = agrpc::GrpcContext::executor_type;

	agrpc::GrpcContext& m_grpc_context;

	void operator()(agrpc::GenericRepeatedlyRequestContext<>&& ctx) const
	{
		auto f = [ctx = std::move(ctx)](asio::yield_context yield)
		{
		};

		asio::spawn(m_grpc_context, f);
	}

	auto get_executor() const noexcept { return m_grpc_context.get_executor(); }
};

and elsewhere I call

agrpc::repeatedly_request(m_service, GenericRequestHandler{m_grpc_context});

Can you spot the problem? I would be grateful with any help.

This won't compile as well

        agrpc::repeatedly_request(m_service,
            asio::bind_executor(
                m_grpc_context.get_executor(),
                [p = this](agrpc::GenericRepeatedlyRequestContext<>&& ctx) mutable
            {
                asio::spawn(p->m_grpc_context, [](auto) { });
            })                                                                                       
        );

By capturing GenericRepeatedlyRequestContext in the lambda, the lambda becomes move-only (because GenericRepeatedlyRequestContext is move-only). You then pass it as lvalue-reference to asio::spawn which then tries to make a copy of it. I don't compile with -fpermissive so I get a different error:

[build] boost/asio/impl/spawn.hpp(1013): error C2280: 'GenericRequestHandler::()::<lambda_1>::<lambda_1>(const GenericRequestHandler::()::<lambda_1> &)': attempting to reference a deleted function

Solution is to move the lambda:

		auto f = [ctx = std::move(ctx)](asio::yield_context yield)  // btw yield_context is essentially just a shared_ptr, you should consider taking it by const&
		{
		};

		asio::spawn(m_grpc_context, std::move(f));   // std::move added here

In more recent versions of asio you also need to provide a completion token to asio:;spawn, example:

asio::spawn(m_grpc_context, std::move(f), asio::detached); 

Your example with repeatedly_request compiles fine for me but looks incomplete? Of course I assume that m_service is of type grpc::AsyncGenericService.

There must be something else, this code also won't compile

        agrpc::repeatedly_request(m_service,
            asio::bind_executor(
                m_grpc_context.get_executor(),
                [this](agrpc::GenericRepeatedlyRequestContext<>&& ctx)
            {
                auto f = [](auto) { };
                asio::spawn(m_grpc_context, std::move(f));
            })
        );

Ok, can you show the surrounding member function and the declaration of the member variables m_service and m_grpc_context?

    agrpc::GrpcContext m_grpc_context;
    grpc::AsyncGenericService m_service;

The member function on the other hand

    void run()                                                                                       
    {   
        agrpc::repeatedly_request(m_service,                                                         
            asio::bind_executor(
                m_grpc_context.get_executor(),
                [this](agrpc::GenericRepeatedlyRequestContext<>&& ctx)                               
            {   
...                               
                });                                                                                  
            })                                                                                       
        );                                                                                                                                                                  
    }

Thank you. You should remove the const from this function:

	auto get_executor() const noexcept { return m_grpc_context.get_executor(); }

I did try this to remove, both from this function and from the operator()(...) but it did not help. I am rewriting the logic with stackless coroutines to I don't have to call spawn.

But how would your code work without const?

Can you give me more context, for example more of the surrounding code? Also what version of asio are you using?

Also more of the error message, I don't think it is just one line?

Try this one:

asio::spawn(m_grpc_context.get_executor(), std::move(f)); 

Looks like older versions of asio have issues identifying GrpcContext as an asio::execution_context.

EDIT: Nevermind, not actually the case

I ended up implementing with stackless coroutines. It less convenient but can make my code more general with async_compose. Don't have much time now to investigate what was the problem. BTW why some completions have signature
void (boo) instead of void (error_code) I will have to write my own error codes?

The void(bool) comes directly from gRPC. The grpc::CompletionQueue only provides a simple bool when an operation completes. The meaning of the bool depends on the function, for example, for agrpc::read:

true indicates that a valid message was read. false when there will be no more incoming messages, either because the other side has called WritesDone() or the stream has failed (or been cancelled).

See https://tradias.github.io/asio-grpc/structagrpc_1_1detail_1_1_read_fn.html#abf1a5078d989f96b013f0f4f8b6d1a4c

Ok. But what about functions like write_and_finish that I suppose perform two grpc function calls? Without an error code I can't possibly know which of the two operations had an error?

write_and_finish is actually just one gRPC call provide by the gRPC library (see here). The meaning of the returned bool is explained in the official documentation and copied into asio-grpc documentation:

true means that the data/metadata/status/etc is going to go to the wire. If it is false, it is not going to the wire because the call is already dead (i.e., canceled, deadline expired, other side dropped the channel, etc).

On the server-side, gRPC does not provide detailed information on why a message couldn't be send to the client. They just give these possible reasons:

This could be because of explicit API cancellation from the client-side or server-side, because of deadline exceeded, network connection reset, HTTP/2 parameter configuration (e.g., max message size, max connection age), etc.

source

Great, thanks.