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

Why I get const correctness problems #79

Closed
criatura2 opened this issue Jul 21, 2023 · 16 comments
Closed

Why I get const correctness problems #79

criatura2 opened this issue Jul 21, 2023 · 16 comments

Comments

@criatura2
Copy link

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.

@criatura2
Copy link
Author

criatura2 commented Jul 21, 2023

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) { });
            })                                                                                       
        );

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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.

@criatura2
Copy link
Author

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));
            })
        );

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

@criatura2
Copy link
Author

    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)                               
            {   
...                               
                });                                                                                  
            })                                                                                       
        );                                                                                                                                                                  
    }

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

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

@criatura2
Copy link
Author

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.

@criatura2
Copy link
Author

But how would your code work without const?

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

@criatura2
Copy link
Author

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?

@Tradias
Copy link
Owner

Tradias commented Jul 21, 2023

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

@criatura2
Copy link
Author

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?

@Tradias
Copy link
Owner

Tradias commented Jul 24, 2023

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

@criatura2
Copy link
Author

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants