ALiwoto / LTW-client

Last Testament Of Wanderers (LTW) Game Client
GNU General Public License v2.0
8 stars 1 forks source link

Don't raise events in separated threads #9

Open ALiwoto opened 3 years ago

ALiwoto commented 3 years ago

Using Task.Run is not acceptable everywhere

    [EditorBrowsable(EditorBrowsableState.Never)]
    protected internal virtual void OnRightUp()
    {
        Task.Run((() =>
        {
            // raise the event in another thread.
            this.RightUp?.Invoke(this, null);
        }));
    }

So what's wrong with this code?

  1. You are creating a new thread, even if the action won't do anything.
  2. You are running every single event on a whole separated thread.

This will cause some problems in the future, such as hug memory leak.


So what to do?

I think something like this will be okay:

    [EditorBrowsable(EditorBrowsableState.Never)]
    protected internal virtual void OnRightUp()
    {
        this.RightUp?.Invoke(this, null);
        if (this.RightUpAsync != null)
        {
            Task.Run((() =>
            {
                // raise the event in another thread.
                this.RightUpAsync?.Invoke(this, null);
            }));
        }
    }

This way, we won't create new threads over and over.