Post(action)/PostDelayed(action) and RemoveCallbacks(action) don't behave as expected

NovacNovac USMember
edited April 2015 in Xamarin.Android

RemoveCallbacks(Action action) doesn't remove the action if that action was posted by itself to the handler.

Example:

            class Test
            {

            Handler handler;
            Action action;

            public Test()
            {
               Handler handler = new Handler();
                Action action = Callback;

            }


            void Callback()
            {
             // repost itself
             handler.PostDelayed(action, 2000);

            // do other stuff
            }


            public void Start()
            {
             handler.PostDelayed(action, 2000);
            }

            public void Stop() 
            {
            handler.RemoveCallbacks(action);
            }



            }

===

var test = new Test();
test.Start();
test.Stop(); 

Stop() doesn't remove "action". Callback still gets called.

I managed to fix this by assigning action a new instance each time.

    void Callback()
    {
    action = new Action(Callback);
     hander.PostDelayed(action, 2000);

    // do other stuff
    }

After doing a bit of reflction, I realized this is probably done to prevent memory leaks. ** PostDelayed(Action action)** creates a RunnableImplementor instance with flag removable = true.

  public bool PostDelayed(Action action, long delayMillis)
    {
      return this.PostDelayed((IRunnable) new Thread.RunnableImplementor(action, **true**), delayMillis);
    }

RunnableImplementor keeps track of all of its instances in a static dictionary.

      private static Dictionary<Action, Thread.RunnableImplementor> instances = new Dictionary<Action, Thread.RunnableImplementor>();

RemoveCallbacks(Action action) asks RunnableImplementor for the instance that corresponds with the passed action so it can delete it.

The problem is, if ** RunnableImplementor** instance is flagged with removable = true, it removes itself from the dictionary when its Run method is called. So after an action is executed, its ** RunnableImplementor** dictionary entry no longer exists and RemoveCallbacks() is unable to delete it.

I expected Posted/PostDelayed(Action) to be C# version of PostDelayed/PostDelayed(IRunnable). It took me sometime to figure this out.

Why not simply remove the dictionary entry before calling Invoke()? This way the action gets deleted then added again and not the other way around.

      public void Run()
      {
        if (this.Handler != null)
          this.Handler.Invoke();

        if (this.removable)
        {
          object obj = (object) Thread.RunnableImplementor.instances;
          bool flag = false;
          try
          {
            Monitor.Enter(obj, ref flag);
            Thread.RunnableImplementor.instances.Remove(this.Handler);
          }
          finally
          {
            if (flag)
              Monitor.Exit(obj);
          }
        }
        this.Dispose();
      }

P.S., As you must have noticed, English is not my first language.

Posts

  • JonathanPryorJonathanPryor USXamarin Team Xamurai

    Your "reposting" of an Action is something that Thread.RunnableImplementor didn't consider, and wasn't designed to support.

    I would suggest that you instead use a Java.Lang.Runnable instance for this use case.

  • NovacNovac USMember

    @JonathanPryor but it should support it, don't you agree?

  • DominicMarierDominicMarier CAMember

    If you call the post or post Delayed and cancel them, there is still 1 that will be called.

    Action myAction = ()=> { Assert(false) };
    var handler = new Handler();
    handler.PostDelayed(myAction, 100);
    handler.PostDelayed(myAction, 100);
    handler.RemoveCallbacks(myAction);
    //myAction will still be called.

    Also, what Novac mentioned prevents the LoopHandler to be used like a Timer.

    Because of crossplatform code, Passing a Action is better.
    This should be fixed, RemoveCallbacks should remove all actions.

  • NovacNovac USMember
    edited April 2015

    @DominicMarier
    Just tested it. RemoveCallbacks(Action) indeed removes only the last callback. This is because RunnableImplementor only keeps track of its last instance.

    Handler:

    class Handler : Java.Lang.Object
    {
    
    ....
    
        public void RemoveCallbacks(Action action)
        {
          //  last created RunnableImplementor instance
          Thread.RunnableImplementor runnableImplementor = Thread.RunnableImplementor.Remove(action);
          if (runnableImplementor == null)
            return;
          this.RemoveCallbacks((IRunnable) runnableImplementor);
          runnableImplementor.Dispose();
        }
    
     ....
    }
    

    RunnableImplementor:

    class RunnableImplementor : Object, IRunnable, IJavaObject, IDisposable
    {
    
    ...
        public RunnableImplementor(Action handler, bool removable)
                : base(JNIEnv.StartCreateInstance("mono/java/lang/RunnableImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
              {
                JNIEnv.FinishCreateInstance(this.Handle, "()V");
                this.Handler = handler;
                this.removable = removable;
                if (!removable)
                  return;
                object obj = (object) Thread.RunnableImplementor.instances;
                bool flag = false;
                try
                {
                  Monitor.Enter(obj, ref flag);
                 Thread.RunnableImplementor.instances.set_Item(handler, this);  // old value is lost
                }
                finally
                {
                  if (flag)
                    Monitor.Exit(obj);
                }
              }
    
    
      public static Thread.RunnableImplementor Remove(Action handler)
          {
            object obj = (object) Thread.RunnableImplementor.instances;
            bool flag = false;
            Thread.RunnableImplementor runnableImplementor;
            try
            {
              Monitor.Enter(obj, ref flag);
    
             // only the last instance is returned
              Thread.RunnableImplementor.instances.TryGetValue(handler, ref runnableImplementor); 
              Thread.RunnableImplementor.instances.Remove(handler);
            }
            finally
            {
              if (flag)
                Monitor.Exit(obj);
            }
            return runnableImplementor;
          }
        }
    
    
    ...
    
    }
    

    I hope these two bugs get fixed soon.

    @JonathanPryor

  • DooksDooks ZAMember ✭✭✭

    Aaaaaaand then nothing happened...

Sign In or Register to comment.