Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#2882 closed defect (duplicate)

ModuleCCSession::rpcCall (C++ version) is mostly unusable

Reported by: jinmei Owned by:
Priority: medium Milestone:
Component: ~Inter-module communication(obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

In #2562, I first thought we could simply use the new rpcCall() in
b10-auth and implemented it, but then lettuce tests constantly failed.
Then I noticed this method has a fundamental issue, and won't
reasonably work at least with some substantial updates.

The problem is that it calls group_recvmsg in its internal Session
object. This internally results in asio::async_read(), but unless it
happens in the context of command or config handler (this is not the
case for #2562, and it would be too fragile to request app be
responsible for it), it breaks an assumption of ASIO: read operation
cannot be nested. (in that sense, it's not specific to rpcCall().
The problem has been there with groupRecvMsg, but we've not relied
on it in practice so far, so we haven't been hit).

I suspect we basically can't mix asynchronous and synchronous
operations at least in the C++ version that uses ASIO. And, in the
case of #2562, we actually shouldn't use the synchronous interface
anyway, because there's no guarantee that this I/O doesn't block.

So my suggestion is: deprecate groupRecvMsg() and rpcCall; they
are not used so far, and, as described above, they generally won't
work.

Subtickets

Change History (3)

comment:1 Changed 7 years ago by vorner

The problem is known. See #2804 ‒ that should make it work.

It still doesn't mean implementing rpcCallAsync and using it in Auth wouldn't be better, but I think we should fix the calls instead of deprecating them.

And, AFAIK, we have similar problem in the python version :-|.

comment:2 Changed 7 years ago by jwright

  • Resolution set to duplicate
  • Status changed from new to closed

comment:3 Changed 7 years ago by shane

  • Milestone New Tasks deleted
Note: See TracTickets for help on using tickets.