Announcement

Collapse
No announcement yet.

Dynamic Multicast Delegate becomes unbound after indeterminate amount of time

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

    Dynamic Multicast Delegate becomes unbound after indeterminate amount of time

    Using UE 4.20.3 with the ROSIntegration plugin, we are creating a simulation for an UGV that we have. The basic premise is that we send ROS messages to the plugin and it handles what to do in the simulation side. We started with a BP model of the vehicle, but in order to speed up the simulation, we migrated the code to a C++ model. We had a separate actor that had the ROS interaction code that we just made a child actor component to both the BP and C++ models. So basically, the ROS actor gets the messages, it processes them and proxies them to the vehicle code to act upon. In order to dispatch an incoming message, we were using a Dynamic Multicast Delegate that the vehicles would bind to. However, after a certain period of time, the delegate reports that it is unbound. We cant figure out why. We've put UPROPERTYs and UFUNCTIONs on everything we can think of, but for some reason it's still becoming unbound. We have a hunch that it's the GC trashing things when we don't want it to, but we have no idea what it is trashing. Any help would be appreciated, and I would be happy to provide more info if I've left off information.

    Code snippets (pruned down to (hopefully) the relevant information)):

    ROS_actor.h
    Code:
    // ... outside our class declaration ...
    
    // Creates our Twist Message Dispatcher Struct
    DECLARE_DYNAMIC_MULTICAST_DELEGATE_TwoParams(FOnTwistMsgDispatched, const FVector&, linear, const FVector&, angular);
    
    // ... inside our class delcaration ...
    public:
      // Activate our Twist Subscriber Function
      UFUNCTION(BlueprintCallable, Category="Twist Subscriber")
      virtual void twistMsgSub();
    
      // Twist Topic name
      UPROPERTY(EditAnywhere, BlueprintReadWrite, Category="ROSCOM", meta=(DisplayName="Twist Subscriber Topic"))
      FString twist_sub_topic_name_;
    
      // Delegate for twist messages
      UPROPERTY(BlueprintAssignable, Category="ROSCOM", meta=(DisplayName="Twist Message Dispatcher"))
      FOnTwistMsgDispatched twist_msg_dispatcher_;
    
    private:
      // Topics for subscribers!
      // These need to be property'd so that they don't get garbage collected
      // https://github.com/code-iai/ROSIntegration/issues/32
      UPROPERTY()
      UTopic* twist_topic_;
    ROS_actor.cpp
    Code:
    void ARosActor::twistMsgSub()
    {
      UROSIntegrationGameInstance* rosinst = Cast<UROSIntegrationGameInstance>(GetGameInstance());
      twist_topic_ = NewObject<UTopic>(UTopic::StaticClass());
      twist_topic_->Init(rosinst->ROSIntegrationCore, twist_sub_topic_name_, TEXT("geometry_msgs/Twist"), 5);
    
      // Create a std::function callback object
      std::function<void(TSharedPtr<FROSBaseMsg>)> TwistCallback = [&](TSharedPtr<FROSBaseMsg> msg) -> void
      {
        auto Concrete = StaticCastSharedPtr<ROSMessages::geometry_msgs::Twist>(msg);
        if (Concrete.IsValid())
        {
          //Unload the data
          FVector linear = FVector(Concrete->linear.x, Concrete->linear.y, Concrete->linear.z);
          FVector angular = FVector(Concrete->angular.x, Concrete->angular.y, Concrete->angular.z);
    
          if (twist_msg_dispatcher_.IsBound())
          {
            twist_msg_dispatcher_.Broadcast(linear, angular);
          }
        }
      };
    
      // Subscribe to the topic
      twist_topic_->Subscribe(TwistCallback);
    }
    Vehicle.h
    Code:
    // .. inside our class declaration ...
    private:
      UFUNCTION()
      void onTwist(const FVector& linear, const FVector& angular);
    
      UPROPERTY()
      UChildActorComponent *ros_comm_;
    Vehicle.cpp
    Code:
    // .. inside BeginPlay() ...
      // Bind our function to the Twist Message Dispatcher, so we get the twist messages.
      ARosActor* ros_actor = dynamic_cast<ARosActor*>(ros_comm_->GetChildActor());
      if (ros_actor != nullptr)
      {
        ros_actor->twist_msg_dispatcher_.AddDynamic(this, &AHusky::onTwist);
      }
    
    // .. outside BeginPlay() ...
    // Our Twist message listener.
    // This is hooked-up in BeginPlay
    void AVehicle::onTwist(const FVector& linear, const FVector& angular)
    {
      this->linear_ = linear;
      this->angular_ = angular;
      this->is_accelerating_ = true;
      clr_cmd_vel_.PlayFromStart(); // Start the timeline to clear these values after a little bit
      // NOTE: This timeline has a single event that, after a little bit, clears the linear_, angular_, and is_accelerating_ values.
    }

    #2
    Originally posted by Parker8283 View Post
    However, after a certain period of time, the delegate reports that it is unbound. We cant figure out why. We've put UPROPERTYs and UFUNCTIONs on everything we can think of, but for some reason it's still becoming unbound.
    How do you know/test that it is unbound?
    https://ravimohan.net/
    https://github.com/ravimohan1991

    Comment


      #3
      Modified the if statement in ROS_actor.cpp to this:

      Code:
      if (twist_msg_dispatcher_.IsBound())
      {
        twist_msg_dispatcher_.Broadcast(linear, angular);
        UE_LOG(LogTemp, Warning, TEXT("Bound"));
      }
      else
      {
        UE_LOG(LogTemp, Warning, TEXT("UNBOUND!!!"));
      }

      Comment


        #4
        I don't have much experience with delegates yet, but I'd check at every Tick if the boolean IsBound() is true and try to pinpoint the events after which set it is to false. Much of the bugs get under scanner if you log the events with high enough frequency.
        https://ravimohan.net/
        https://github.com/ravimohan1991

        Comment


          #5
          UObject lifecycle is not what you think it is in Unreal. Don't use the Constructor to setup stuff like this, use an Init function of some kind instead when the actor is actually created in the game world. In addition:

          - Don't create UObjects in the constructor with NewObject, as it isn't safe. You need to create them as DefaultSubObjects with the ObjectInitializer if you want them to be permanent members of the class. Look at the engines/example projects actors for examples of how they create sub-objects.

          - Don't use std library. Not only is it not portable, but Unreal has it's own delegate stuff anyway. FSimpleDelegate::CreateUObject is probably what you want.

          Comment


            #6
            One thing I noticed is this:
            Code:
            twist_topic_ = NewObject<UTopic>(UTopic::StaticClass());
            Be aware that UTopic::StaticClass() is being used as the Outer parameter of NewObject there, this version of NewObject:

            Code:
            T* NewObject(UObject* Outer = (UObject*)GetTransientPackage())
            So be sure to change that to
            Code:
            NewObject<UTopic>(this, UTopic::StaticClass());
            seeing as the actor (this) is a valid outer to manage this object. I'm not sure if that is the cause of the bug, but it might be since the outer ties into garbage collection.

            Something else you can try to confirm whether it's garbage collection or not, is to call twist_topic_->AddToRoot() after creation it, which will ensure it won't be garbage collected but you must call twist_topic_->RemoveFromRoot() when you're done with it. I would advise to try that only for debugging reasons, as the more appropriate approach would be to have it managed via a valid Outer and UPROPERTY() like you've been doing.
            Journeyman's Minimap - Available on Marketplace - Forum topic - Video

            Comment


              #7
              TheJamsh I'm a little confused about your first comment, because none of that code is in my constructors. The second comment, I was seeing that too, but I was following the tutorials on the ROSIntegration plugin, so I just left it as it works with other topics we are making, just not this one with the dispatcher. And for the third comment, I have to use std::function because that's what the library for the ROSIntegration plugin requires. That's out of my hands.

              Zhi Kang Shao I did not know about this, I was just following the ROSIntegration plugin's Quick Start advice. In the end, I think I'll do the usual CreateDefaultSubobject.

              To all, after further analysis this morning, we think something is happening because we have the delegate within a lambda expression, and the UE GC sees us no longer using it within the rest of the normal codebase (it might not be able to see into lambdas with them not being UFUNCTIONs?), so it then trashes it. Trying to be smart or whatever. We don't know for 100% certainty, but we also think moving the ROS code into our normal actor instead of having a separate actor that becomes a child actor component of the first actor would help to avoid some of the problems and remove the need for the delegate in the first place. Also a cleaner codebase because we don't have weird casting/proxy calls. However, we would still like to find the solution, just in case anyone else might run into this problem and doesn't have the same setup as we do. Just thinking of everyone else.

              Thank you all for you answers and work so far!
              Last edited by Parker8283; 06-10-2019, 10:55 AM.

              Comment

              Working...
              X