alec1o / Netly

Cross-Platform and Multi-Protocol C# Socket Library. (Extremely fast and easy) 🇳 🇪 🇹 🇱 🇾
https://netly.docs.kezero.com
MIT License
58 stars 8 forks source link

MainThread.Clean does not remove all callbacks per call #21

Closed Karabaev closed 11 months ago

Karabaev commented 1 year ago

I believe there is bug in the MainThread.Clean method. It removes only half callbacks per one call if there are many callbacks

alec1o commented 1 year ago

Hi sorry for the delay! well could you show me the example script? The clean method only cleans up the returns that are currently stored and this method must be called several times. what I think is happening is that netly works with ThreadPool and the events are not instantaneous and what must have happened is that you are calling clean before the event is in the event list: look at the implementation below:

using System;
using System.Collections.Generic;

namespace Netly.Core
{
    /// <summary>
    /// Netly: MainThread
    /// </summary>
    public static class MainThread
    {
        /// <summary>
        /// Automatic clean callbacks
        /// </summary>
        public static bool Automatic { get; set; } = true;
        private static List<Action> _callbacks { get; set; } = new List<Action>();

        /// <summary>
        /// Add callback to execute on (Main/Own)Thread
        /// </summary>
        /// <param name="callback">callback</param>
        public static void Add(Action callback)
        {
            if (callback == null) return;

            if (Automatic)
                callback.Invoke();
            else
                _callbacks.Add(callback);
        }

        /// <summary>
        /// Use to clean/publish callbacks: work if (Automatic is false)
        /// </summary>
        public static void Clean()
        {
            if (Automatic is false && _callbacks.Count > 0)
            {
                for (int i = 0; i < _callbacks.Count; i++)
                {
                    _callbacks[0].Invoke();
                    _callbacks.RemoveAt(0);
                }
            }
        }
    }
}

Anyway, I didn't put lock on the callbacks so I wouldn't have a performance problem if I received thousands of messages per second, if I put lock on I would make the messages wait for the list to be dispatched. well, that doesn't solve the problem! what i think is that it doesn't dispatch due to the ThreadPool delay when receiving an event and adding it to the event list bearing in mind that the loop is called several times per second this shouldn't be a problem.

alec1o commented 11 months ago

Hello, I found the cause of the problem and already fixed it

Notice: It wasn't MainThread, it was Package.cs actually MessageFraming.cs.

Describing the cause: What happened is that when a raw buffer (data) is received, it would go through Package.cs and Package.cs has the methods to parse and dispatch the messages. What caused this error is that TCP is a streaming system, so when Package.cs received more than one piece of data in the same buffer, the first piece of data received was always dispatched and the rest were stuck in the list of buffers, So there were many messages in the buffer list, and when a buffer was received it was not dispatched but rather remained at the end of the queue, and the oldest data was dispatched, thus bringing about the described effect (this would cause a memory leak).

Solution: needed some kind of recrusivity where: when a data was dispatched it calls a function to dispatch, and the function will dispatch it will dispatch the other data in the buffer if it exists, then the data is locked due to lack of this functionality (recrucivity)... sorry for not finding the problem earlier! But now everything is ready Netly >= 3.0