Concurrency - Lambda based message passing

[LogicParrot]

Hello, I've been thinking about ways to eliminate threading / locking issues, and I had this idea.

Forgive me if this has been discussed before. Also, if the technique I'm about to describe has a common name in the C++ world, I'd be glad to hear it. I've done some searching but found no such name.

The idea is based on old school message passing, but it's enhanced with lambdas.
Reminder of old-school message-passing: Each thread has a single inbox, similar to our World::m_PlayersToAdd. Other threads can send messages to that inbox. The thread periodically checks the inbox, and acts accordingly. The neat thing here is that only one single lock is required per thread (to prevent reading and writing to the inbox at the same time), and concurrency is very clean.

Traditionally, the downside is that one would need to create some sort of messaging system. e.g. each message has an ID which the consuming thread reads and determines the message type accordingly, etc.

With lambdas, this is no longer a problem: Each thread's inbox is a queue of lambdas, threads send lambdas to each other, concurrency is perfect, code is clean.

Example pseudocode for transferring an entity from one world thread to another world thread:

cEntity::NewDoMoveToWorld(cWorld & a_NewWorld)
{
  ...Code here...
   m_World.RemoveEntity(this);
   a_NewWorld.message([this](cWorld & a_World) -> void
  {
     a_World.AddEntity(this); // This is threadsafe, because the world will execute it when it processes the inbox.
  }
}

Example pseudocode for a cClientHandle removing a player safely (Currently player removal is NOT thread safe, see this)

cClientHandle:: Destroy()
{
  ...Code here...
   m_Player.GetWorld().message([m_Player](cWorld & a_World) -> void
  {
     a_World.RemovePlayer(m_Player);
  }
}

Example inbox processing, that's just about all the concurrency code we need:

cworld::tick()
{
  ...Code here...
  {
     cCSLock Lock(m_InboxLock);
     for (message : m_Inbox)
     {
        message(*this);
     }
    if (Inbox.size() > 0) Inbox.clear();
  }
}

cWorld::message(std::function<void(cWorld & a_World)> a_Message)
{
   cCSLock Lock(m_InboxLock);
   m_Inbox.push_back(a_Message);
}

Thoughts?

[LogicParrot]

The example cworld::tick() can be optimized by copying the queue and releasing the lock asap. But that's beside the point.

[xoft]

Such a system would mostly work, but sometimes just passing messages is not adequate. Consider the cases when you need to ask something: "Is XY available?" or "Is there a player Z already connected?" You need to wait for the result somehow, and that's what actually introduces all the deadlocks.

Note that it would be quite difficult, if not impossible, to rewrite some of these. Checking the connecting player for duplicate name needs to be done before the player connection code is allowed to continue, and just scheduling everything would make the resulting code unreadable.

This technique is already used where it makes sense, such as queueing block updates in a world.

[LogicParrot]

But this simple system also works for returned results: Make the other process send a message back via a nested lambda. Deadlock nonexistence guaranteed.

cRoot::foo would like to access cWorld::isNameDupe:

void cRoot::foo(cPlayer & a_Player)
{
    a_Player.GetWorld()->message([this, a_Player](cWorld & a_World){
      if (a_World.IsNameDupe(a_Player.GetName()))
      {
          this.message([a_Player](cRoot & a_Root){
            a_Player->myn(); // or whatever.
            a_Player->qwerty(5, 20); // or whatever.
          });
      }
      else
      {
         this.message([a_Player](cRoot & a_Root){
           a_Player.xyz()  // or whatever
           a_Player.abc(a_Root.ikj());  // or whatever.
           a_Root.etc(); // or whatever.
         });
      }
   });
}

[xoft]

Problems:
- The player disconnects before the world thread processes the message queue
- The player object needs to fracture all its actions into tiny bits, such as "ContinueLogin()". A hundred tiny functions and no-one knows when to call that specific one
- The player object receives data from the client while it's waiting for confirmation, what should it do with the data?
- Plugin decides to kick the player before the messages are processed, now the player object is in "being kicked" state and suddenly receives a "continue login" message. Each message handler must be made to handle any such state -> blown up error checking.

[xoft]

Also, please use the shcode=cpp tag instead of the code tag.

[LogicParrot]

I can see a disadvantage though: This spans two ticks, But I suppose that's acceptable in many cases, isn't it?

[LogicParrot]

- The player object needs to fracture all its actions into tiny bits, such as "ContinueLogin()". A hundred tiny functions and no-one knows when to call that specific one

No! Not at all! You just fill the code right where I wrote ContinueLogin(). This is not an extra function. Sorry for being vague there.

Edit: I replaced ContinueLogin to avoid further confusion.

Let me think about the rest of your points.

[LogicParrot]

I see. So I fixed deadlocks at the cost of out-of-order hell.

Is this still viable for unifying queues in the non-returning case? For instance, cWorld has lots of queues: `m_ClientstoAdd`, `clientsToRemove`, etc. (And now we also need m_PlayersToRemove). Those can all be combined to a single queue of lambdas.

It's tidier. (and performance-wise, it might even be a tiny bit faster, because the queues are typically empty, and when they're empty, the number of queue checks are as much as there are queues. )

[xoft]

Unless there's a lot of potential in the rewrite, I'm against it - too much of a change for very little benefit. The performance is already good enough, and the deadlocks happen in other parts of the code. Not sure about the tidier part either, now it's tidy because it's wrapped into functions with clear names and the implementation details (such as moving it to a queue for later processing) is hidden from the client classes.