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

Service request failing #563

Open
emanodus opened this issue Oct 6, 2024 · 2 comments
Open

Service request failing #563

emanodus opened this issue Oct 6, 2024 · 2 comments

Comments

@emanodus
Copy link

emanodus commented Oct 6, 2024

I am using the foxy branch.

I have a pair of states and a pair of transition to move from one to another. I want to send a service request at the exit of each state and entry of each state to another process hosting their service servers. I have kept the service request calls in the onEntry and onExit methods of both the states. Though the onEntry service request is processed successfully and a response is recieved, the onExit service call's are blocked indefinetly at the client side. On the server side for the service inside onExit, the callback exits gracefully, but no response is recived in the statemachine process.

Are we not meant to send service requests from the onExit methods of a state?

Also if I have a updatable state (multiple inheritance from SmaccStateBase and IsmaccUpdateable), even then I can not recive a response for the request sent from within the update function attached to the updatable state. It just blocks indefinitely.

@emanodus
Copy link
Author

emanodus commented Oct 6, 2024

On firther debugging, there were competing mutex lock

  1. std::lock_guard<std::recursive_mutex> lock(this->getStateMachine().getMutex());

    2.
    std::lock_guard<std::recursive_mutex> lock(smaccStateMachine_->m_mutex_);

I resolved it by scoping the mutex lock in 1 as

 void exit()
  {
    auto * derivedThis = static_cast<MostDerived *>(this);
    this->getStateMachine().notifyOnStateExitting(derivedThis);
    {
      {
        std::lock_guard<std::recursive_mutex> lock(this->getStateMachine().getMutex());
        this->getStateMachine().notifyOnStateExitting(derivedThis);
      }
      
      try
      {
        {
        std::lock_guard<std::recursive_mutex> lock(this->getStateMachine().getMutex());
        TRACEPOINT(smacc2_state_onExit_start, STATE_NAME);
        }
        // static_cast<MostDerived *>(this)->onExit();
        standardOnExit(*derivedThis);
        {
        std::lock_guard<std::recursive_mutex> lock(this->getStateMachine().getMutex());
        TRACEPOINT(smacc2_state_onExit_end, STATE_NAME);

        }
      }
      catch (...)
      {
      }
      std::lock_guard<std::recursive_mutex> lock(this->getStateMachine().getMutex());
      this->getStateMachine().notifyOnStateExited(derivedThis);
    }
  }

I am afraid it will cause unwanted side effects or this might not be the most elegant way to break the deadlock. Please let me know if this modification is in coherence with the design phiosophy?

@yassiezar
Copy link
Contributor

Hi @emanodus

This is a similar issue to the one reported in #556. The issue was introduced due to a bad merge, so your solution is likely sound. Indeed, its similar to the one that was implemented in Humble (though its not been backported to Foxy yet). We'll look into backporting it soon

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