hw: re-work the ipc node's internal state machine

* Split the internal state into incoming and outgoing message relations
* Avoid fragmenting of one state like formerly '_state' and '_help'
* Remove pointer to caller, use incoming FIFO instead

This commit fixes at least two bugs that were triggered by tests that
destroy threads in many different states, like run/bomb:

* The '_help' data member was not reset reliable in each situation where a
  helping relationship came to an end. However, when we fixed this bug alone
  in the old state model, the issues remained. The new state model fixes
  this bug as well.

* A thread sometimes referenced an already dead thread as receiver. This caused
  the kernel IPC code to access the vtable of an object that didn't exist any
  longer. Note that the two threads were not in direct IPC relationship while
  the receiver was destroyed, so, there must have been an intermediate node
  between them. Due to the complexity of this problem, we eventually gave up
  pin-pointing the exact reason in the kernel IPC code. The issue disappeared
  with the new state model.

Fix genodelabs/genode#4704
This commit is contained in:
Stefan Kalkowski 2022-12-16 11:53:27 +01:00 committed by Christian Helmuth
parent 5a558a64e1
commit fc690f1c47
2 changed files with 110 additions and 175 deletions

View File

@ -25,153 +25,107 @@
using namespace Kernel;
void Ipc_node::_receive_request(Ipc_node &caller)
void Ipc_node::_receive_from(Ipc_node &node)
{
_thread.ipc_copy_msg(caller._thread);
_caller = &caller;
_state = INACTIVE;
_thread.ipc_copy_msg(node._thread);
_in.state = In::REPLY;
}
void Ipc_node::_receive_reply(Ipc_node &callee)
void Ipc_node::_cancel_send()
{
_thread.ipc_copy_msg(callee._thread);
_state = INACTIVE;
_thread.ipc_send_request_succeeded();
}
if (_out.node) {
/*
* If the receiver is already processing our message,
* we have to ensure that he skips sending a reply by
* letting his node state indicate our withdrawal.
*/
if (_out.node->_in.state == In::REPLY)
_out.node->_in.queue.head([&] (Queue_item &item) {
if (&item == &_queue_item)
_out.node->_in.state = In::REPLY_NO_SENDER;
});
void Ipc_node::_announce_request(Ipc_node &node)
{
/* directly receive request if we've awaited it */
if (_state == AWAIT_REQUEST) {
_receive_request(node);
_thread.ipc_await_request_succeeded();
return;
_out.node->_in.queue.remove(_queue_item);
_out.node = nullptr;
}
/* cannot receive yet, so queue request */
_request_queue.enqueue(node._request_queue_item);
}
void Ipc_node::_cancel_request_queue()
{
_request_queue.dequeue_all([] (Queue_item &item) {
Ipc_node &node { item.object() };
node._outbuf_request_cancelled();
});
}
void Ipc_node::_cancel_outbuf_request()
{
if (_callee) {
_callee->_announced_request_cancelled(*this);
_callee = nullptr;
if (_out.sending()) {
_thread.ipc_send_request_failed();
_out.state = Out::READY;
}
}
void Ipc_node::_cancel_inbuf_request()
bool Ipc_node::_helping() const
{
if (_caller) {
_caller->_outbuf_request_cancelled();
_caller = nullptr;
return _out.state == Out::SEND_HELPING && _out.node;
}
bool Ipc_node::can_send_request() const
{
return _out.state == Out::READY && !_in.waiting();
}
void Ipc_node::send_request(Ipc_node &node, bool help)
{
node._in.queue.enqueue(_queue_item);
if (node._in.waiting()) {
node._receive_from(*this);
node._thread.ipc_await_request_succeeded();
}
}
void Ipc_node::_announced_request_cancelled(Ipc_node &node)
{
if (_caller == &node)
_caller = nullptr;
else
_request_queue.remove(node._request_queue_item);
}
void Ipc_node::_outbuf_request_cancelled()
{
if (_callee == nullptr)
return;
_callee = nullptr;
_state = INACTIVE;
_thread.ipc_send_request_failed();
}
bool Ipc_node::_helps_outbuf_dst()
{
return (_state == AWAIT_REPLY) && _help;
}
bool Ipc_node::can_send_request()
{
return _state == INACTIVE;
}
void Ipc_node::send_request(Ipc_node &callee, bool help)
{
_state = AWAIT_REPLY;
_callee = &callee;
_help = false;
/* announce request */
_callee->_announce_request(*this);
_help = help;
_out.node = &node;
_out.state = help ? Out::SEND_HELPING : Out::SEND;
}
Thread &Ipc_node::helping_sink()
{
return _helps_outbuf_dst() ? _callee->helping_sink() : _thread;
return _helping() ? _out.node->helping_sink() : _thread;
}
bool Ipc_node::can_await_request()
bool Ipc_node::can_await_request() const
{
return _state == INACTIVE;
return _in.state == In::READY;
}
void Ipc_node::await_request()
{
_state = AWAIT_REQUEST;
_request_queue.dequeue([&] (Queue_item &item) {
_receive_request(item.object());
_in.state = In::WAIT;
_in.queue.head([&] (Queue_item &item) {
_receive_from(item.object());
});
}
void Ipc_node::send_reply()
{
/* reply to the last request if we have to */
if (_state == INACTIVE && _caller) {
_caller->_receive_reply(*this);
_caller = nullptr;
}
if (_in.state == In::REPLY)
_in.queue.dequeue([&] (Queue_item &item) {
Ipc_node &node { item.object() };
node._thread.ipc_copy_msg(_thread);
node._out.node = nullptr;
node._out.state = Out::READY;
node._thread.ipc_send_request_succeeded();
});
_in.state = In::READY;
}
void Ipc_node::cancel_waiting()
{
switch (_state) {
case AWAIT_REPLY:
_cancel_outbuf_request();
_state = INACTIVE;
_thread.ipc_send_request_failed();
break;
case AWAIT_REQUEST:
_state = INACTIVE;
if (_out.sending())
_cancel_send();
if (_in.waiting()) {
_in.state = In::READY;
_thread.ipc_await_request_failed();
break;
return;
default: return;
}
}
@ -184,8 +138,12 @@ Ipc_node::Ipc_node(Thread &thread)
Ipc_node::~Ipc_node()
{
_cancel_request_queue();
_cancel_inbuf_request();
_cancel_outbuf_request();
}
_in.state = In::DESTRUCT;
_out.state = Out::DESTRUCT;
_cancel_send();
_in.queue.for_each([&] (Queue_item &item) {
item.object()._cancel_send();
});
}

View File

@ -35,74 +35,56 @@ class Kernel::Ipc_node
using Queue_item = Genode::Fifo_element<Ipc_node>;
using Queue = Genode::Fifo<Queue_item>;
enum State
struct In
{
INACTIVE = 1,
AWAIT_REPLY = 2,
AWAIT_REQUEST = 3,
enum State { READY, WAIT, REPLY, REPLY_NO_SENDER, DESTRUCT };
State state { READY };
Queue queue { };
bool waiting() const
{
return state == WAIT;
}
};
struct Out
{
enum State { READY, SEND, SEND_HELPING, DESTRUCT };
State state { READY };
Ipc_node *node { nullptr };
bool sending() const
{
return state == SEND_HELPING || state == SEND;
}
};
Thread &_thread;
Queue_item _request_queue_item { *this };
State _state { INACTIVE };
Ipc_node *_caller { nullptr };
Ipc_node *_callee { nullptr };
bool _help { false };
Queue _request_queue { };
Queue_item _queue_item { *this };
Out _out { };
In _in { };
/**
* Buffer next request from request queue in 'r' to handle it
* Receive a message from another IPC node
*/
void _receive_request(Ipc_node &caller);
void _receive_from(Ipc_node &node);
/**
* Receive a given reply if one is expected
* Cancel an ongoing send operation
*/
void _receive_reply(Ipc_node &callee);
void _cancel_send();
/**
* Insert 'r' into request queue, buffer it if we were waiting for it
* Return wether this IPC node is helping another one
*/
void _announce_request(Ipc_node &node);
bool _helping() const;
/**
* Cancel all requests in request queue
*/
void _cancel_request_queue();
/**
* Cancel request in outgoing buffer
*/
void _cancel_outbuf_request();
/**
* Cancel request in incoming buffer
*/
void _cancel_inbuf_request();
/**
* A request 'r' in inbuf or request queue was cancelled by sender
*/
void _announced_request_cancelled(Ipc_node &node);
/**
* The request in the outbuf was cancelled by receiver
*/
void _outbuf_request_cancelled();
/**
* Return wether we are the source of a helping relationship
*/
bool _helps_outbuf_dst();
/**
* Make the class noncopyable because it has pointer members
* Noncopyable
*/
Ipc_node(const Ipc_node&) = delete;
/**
* Make the class noncopyable because it has pointer members
*/
const Ipc_node& operator=(const Ipc_node&) = delete;
public:
@ -120,11 +102,11 @@ class Kernel::Ipc_node
/**
* Send a request and wait for the according reply
*
* \param callee targeted IPC node
* \param help wether the request implies a helping relationship
* \param node targeted IPC node
* \param help wether the request implies a helping relationship
*/
bool can_send_request();
void send_request(Ipc_node &callee,
bool can_send_request() const;
void send_request(Ipc_node &node,
bool help);
/**
@ -137,15 +119,10 @@ class Kernel::Ipc_node
*/
template <typename F> void for_each_helper(F f)
{
/* if we have a helper in the receive buffer, call 'f' for it */
if (_caller && _caller->_help)
f(_caller->_thread);
/* call 'f' for each helper in our request queue */
_request_queue.for_each([f] (Queue_item &item) {
_in.queue.for_each([f] (Queue_item &item) {
Ipc_node &node { item.object() };
if (node._help)
if (node._helping())
f(node._thread);
});
}
@ -155,7 +132,7 @@ class Kernel::Ipc_node
*
* \return wether a request could be received already
*/
bool can_await_request();
bool can_await_request() const;
void await_request();
/**
@ -168,7 +145,7 @@ class Kernel::Ipc_node
*/
void cancel_waiting();
bool awaits_request() const { return _state == AWAIT_REQUEST; }
bool awaits_request() const { return _in.waiting(); }
};
#endif /* _CORE__KERNEL__IPC_NODE_H_ */