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

Race condition: Mailbox think the target machine is busy when it's not #11

Open
huan opened this issue Jan 11, 2022 · 0 comments
Open
Labels
bug Something isn't working

Comments

@huan
Copy link
Member

huan commented Jan 11, 2022

In our Mailbox actor protocol, the target machine must send CHILD_IDLE to Mailbox for reporting it's in idle state.

However, there has a situation that, the target machine is in state idle, but the Mailbox think it's not in idle:

  1. The target machine received a new message when it's idle, the Mailbox set it as busy
  2. The target has a state with always: States.idle transition back to idle, with its entry use Mailbox.Actions.reply() (it's just a sendParent wrapper) to send a message to the original message sender (actor)
  3. If that message received by the origin message sender(actor), triggered another message sent as a reply to the Mailbox, then this race condition will happen:
    1. The target machine is in idle state, but the Mailbox.Actions.idle() in the entry of the idle state (a wrapper of sendParent('CHILD_IDLE')) has not been executed yet: it's in the actions list and wait for the return of the previous Mailbox.Actions.reply() to return
    2. The mailbox has not received the CHILD_IDLE so it thinks the target is in the busy state
    3. The target actually can receive the new message, which makes this new message pass-through the mailbox, without setting the current message origin, which means this message can not be replied to in the future.
    4. If the target machine replies to this new message, then the reply message will be sent to the previous message, which is not right.

This is a 4+ hours debugging journey.

Workaround 1

Use after with a timeout of 0 (like setImmediate) to put the future tasks at the end of the event queue, so that the Mailbox.Actions.idle()(wrapper of actions.sendParent('CHILD_IDLE')) can be executed before it.

  [Types.CONTACTS]: {
    actions: [
      actions.assign({ contacts: (_, e) => e.payload.contacts }),
    ],
-     always: States.registered,
+     after: {
+       0: States.registered,
+     },
  },

Workaround 2

Use delay options when sending reply events in Mailbox.Actions.reply to put the action as a task in the future event loop:

const reply: typeof actions.sendParent = (event, options) => {
+   const normalizedOptions: SendActionOptions<any, any> = {
+     delay: 0,
+     ...options,
+   }

  if (typeof event === 'function') {
    return actions.sendParent(
      (ctx, e, meta) => Events.CHILD_REPLY(event(ctx, e, meta)),
-        options,
+       normalizedOptions,
    )
  } else if (typeof event === 'string') {
    return actions.sendParent(
      Events.CHILD_REPLY({ type: event }),
-        options,
+       normalizedOptions,
    )

Related discussion

@huan huan added the bug Something isn't working label Jan 11, 2022
huan added a commit that referenced this issue Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant