[help] C# and multithreading

[jsmorley]

I think i can easy fix that. My method with switch can handle with it. I'll make it tomorrow if you don't mind.

I'm in no big hurry at all.

[jsmorley]

P.S. FinishAction is still lagging one update behind in the latest version. If I use OnChangeAction in place of FinishAction in that skin I just posted, then it works fine. The difference of course is that OnChangeAction or OnUpdateAction take place in the skin / Rainmeter code, and FinishAction takes place in the plugin code.

However, you can't always use OnChangeAction, since you may want to take some action whenever the plugin "finishes the thread", even if the returned value is still the same.

FinishAction is not the most important thing here though...

[jsmorley]

BTW, here is the latest plugin code in case any of the other codeheads want to look at it.

Code: Select all

using System;
using System.Collections.Generic;
using System.Net;
using System.Net.NetworkInformation;
using System.Threading;
using Rainmeter;

namespace PluginCheckNet
{
    internal class Measure
    {
        public IntPtr SkinHandle;
        public string ConnectionType;
        public double ReturnValue;
        public int UpdateCounter;
        public int UpdateRate;
        private string _finishAction;

        static Thread _networkThread;
        private static RulyCanceler canceler;

        public void FinishAction()
        {
            if (!String.IsNullOrEmpty(_finishAction))
            {
                API.Execute(SkinHandle, _finishAction);
            }
        }

        private void CheckConnection(string type, RulyCanceler c)
        {
            while (true)
            {
                c.ThrowIfCancellationRequested();

                #region code
                if ((string)type == "NETWORK" || (string)type == "INTERNET")
                {
                    if (Convert.ToDouble(NetworkInterface.GetIsNetworkAvailable()) == 0)
                    {
                        ReturnValue = -1.0;
                    }
                    else
                    {
                        ReturnValue = 1.0;
                    }
                }

                if (ReturnValue == 1.0 && (string)type == "INTERNET")
                {
                    try
                    {
                        IPAddress[] addresslist = Dns.GetHostAddresses("www.msftncsi.com");

                        if (addresslist[0].ToString().Length > 6)
                        {
                            ReturnValue = 1.0;
                        }
                        else
                        {
                            ReturnValue = -1.0;
                        }
                    }
                    catch
                    {
                        ReturnValue = -1.0;
                    }
                }
                #endregion

                FinishAction();
                canceler.Cancel();
            }

            //Thread.CurrentThread.Abort(); // Never end a thread in the main Update() function.
        }

        internal Measure()
        {
        }

        internal void Reload(Rainmeter.API rm, ref double maxValue)
        {
            SkinHandle = rm.GetSkin();
            ConnectionType = rm.ReadString("ConnectionType", "INTERNET").ToUpperInvariant();
            _finishAction = rm.ReadString("FinishAction", "");
            if (ConnectionType != "NETWORK" && ConnectionType != "INTERNET")
            {
                API.Log(API.LogType.Error, "CheckNet.dll: ConnectionType=" + ConnectionType + " not valid");
            }

            UpdateRate = rm.ReadInt("UpdateRate", 20);
            if (UpdateRate <= 0)
            {
                UpdateRate = 20;
            }
        }

        internal double Update()
        {
            if (UpdateCounter == 0)
            {
                if (ConnectionType == "NETWORK" || ConnectionType == "INTERNET")
                {
                    if (_networkThread == null || _networkThread.ThreadState == ThreadState.Stopped)
                    //We check here to see if all existing instances of the thread have stopped,
                    //and start a new one if so.
                    {
                        canceler = new RulyCanceler();
                        _networkThread = new Thread(() =>
                        {
                            try
                            {
                                CheckConnection(ConnectionType, canceler);
                            }
                            catch (OperationCanceledException) { }
                        });
                        _networkThread.Start();
                    }
                }
            }

            UpdateCounter = UpdateCounter + 1;
            if (UpdateCounter >= UpdateRate)
            {
                UpdateCounter = 0;
            }

            return ReturnValue;
        }

        //internal string GetString()
        //{
        //    return "";
        //}

        //internal void ExecuteBang(string args)
        //{
        //}

        //it is recommended that this Dispose() function be in all examples,
        //and called in Finalize().

        internal static void Dispose()
        {
            if (_networkThread.IsAlive)
                canceler.Cancel();
        }
    }

    class RulyCanceler
    {
        object _cancelLocker = new object();
        bool _cancelRequest;
        bool IsCancellationRequested
        {
            get { lock (_cancelLocker) return _cancelRequest; }
        }

        public void Cancel() { lock (_cancelLocker) _cancelRequest = true; }

        public void ThrowIfCancellationRequested()
        {
            if (IsCancellationRequested) throw new OperationCanceledException();
        }
    }

    static class Plugin
    {
        static Dictionary<uint, Measure> Measures = new Dictionary<uint, Measure>();

        [DllExport]
        public unsafe static void Finalize(void* data)
        {
            Measure.Dispose();
            uint id = (uint)data;
            Measures.Remove(id);
        }

        [DllExport]
        public unsafe static void Initialize(void** data, void* rm)
        {
            uint id = (uint)((void*)*data);
            Measures.Add(id, new Measure());
        }
        [DllExport]
        public unsafe static void Reload(void* data, void* rm, double* maxValue)
        {
            uint id = (uint)data;
            Measures[id].Reload(new Rainmeter.API((IntPtr)rm), ref *maxValue);
        }

        [DllExport]
        public unsafe static double Update(void* data)
        {
            uint id = (uint)data;
            return Measures[id].Update();
        }

        //[DllExport]
        //public unsafe static char* GetString(void* data)
        //{
        //    uint id = (uint)data;
        //    fixed (char* s = Measures[id].GetString()) return s;
        //}

        //[DllExport]
        //public unsafe static void ExecuteBang(void* data, char* args)
        //{
        //    uint id = (uint)data;
        //    Measures[id].ExecuteBang(new string(args));
        //}
    }

}

[jsmorley]

I think i can easy fix that. My method with switch can handle with it. I'll make it tomorrow if you don't mind.

I'm sorta ok with going back to a switch approach with separate threads for "network" and "internet" if that is really required, but I'm stumped as to why it should be so. The approach we have now works fine as long as we only have one instance of the plugin in a skin running at one time, and if we have created a situation where two instances of the plugin in different measures in a skin are "conflicting" with each other, that can't be accepted. We can have a dozen parent WebParser measures in the same skin, and they just don't know that any others even exist.

My feeling is that even if we can find ways that will enable THIS plugin to work ok, they need to be "solutions" and not "bandaids" that are specific to the functionality in this plugin.

I think we have some kind of "race condition" or other conflict between the overall Rainmeter thread executing the plugin, and the thread we are creating in the plugin. I think we need to look at some kind of Mutex or CRITICAL_SECTION solution.

[FlyingHyrax]

Hate to be a bother, but I'm a little confused about something:

Code: Select all

private void CheckConnection(string type, RulyCanceler c)
{
...
canceler.Cancel();
...
}

Why does this work? When threaded, CheckConnection is running in its own stack. The only reason it has a reference to our instance of RulyCanceler is because we passed one to the method (c). Shouldn't it be c.Cancel()? canceler.Cancel() doesn't seem to throw an error, but why wouldn't it? canceler is a reference to a RulyCanceler object, sitting out on the heap somewhere, and the reference (not the object) is in the stack for the main thread, not _networkThread.
More reading. Class-level variables are shared among threads.
So now my question is - why do we bother passing RulyCanceler c to the method at all if we can just reference that object with canceler?



-----
Just putting this here for future reference. I feel as though in principle it shouldn't be needed, but it might make a nice failsafe to ensure that secondary threads started by the plugin will be terminated if the plugin exits unexpectedly?
http://msdn.microsoft.com/en-us/library/system.threading.thread.isbackground%28v=vs.110%29.aspx
-----
Another thing for future reference:

Note that you don't have to stop or free the thread. This is done automatically by the .NET Framework common language runtime.

It may be that we aren't going to get the same precise control over thread lifetimes in C# that you can (I guess) achieve in C++, because in the end we can't actually control thread termination?

[Aragas]

We can't have one option that will be for all ConnectionTypes. Rainmeter can check option whenever he want. But, in this moment the plugin can check other option. So, the ReturnValue will be not what Rainmeter want. This is bad. So yet each option is splitted. We have yet InternetAvailable and NetworrkAvailable.
Rainmeter can make a call "Give me internet status" and then "Give me network status". The plugin will give back separate option, not one - ReturnValue.
Check my fork.
Aaand, don't use it. Have discovered bugs.Lol. UpdateCounter was on the top.

So now my question is - why do we bother passing RulyCanceler c to the method at all if we can just reference that object with canceler?

My bad. Fixed, thanks.

[jsmorley]

We can't have one option that will be for all ConnectionTypes. Rainmeter can check option whenever he want. But, in this moment the plugin can check other option. So, the ReturnValue will be not what Rainmeter want. This is bad. So yet each option is splitted. We have yet InternetAvailable and NetworrkAvailable.
Rainmeter can make a call "Give me internet status" and then "Give me network status". The plugin will give back separate option, not one - ReturnValue.
Check my fork.
Aaand, don't use it. Have discovered bugs.

I really don't think separating them is needed.

I sorta started over, as I think things got a little messy, See my repository for where I am right now. It works just fine, but I need to do two things:

1) Protect against race conditions if the measure is set to DynamicVariables=1 (which will cause Reload() to execute on every measure update, potentially causing a change in variables like ConnectionType while Update() is still running.

2) Deal with Refresh / Unload in a graceful way.

[jsmorley]

You really must not use "static" in the context of a plugin, or it becomes "global" and you cause conflicts with other measures or even other skins using the same plugin.

That was the root of most of the problems I was having yesterday. It is also the root of why you feel you need to separate them.

[FlyingHyrax]

1) Protect against race conditions if the measure is set to DynamicVariables=1 (which will cause Reload() to execute on every measure update, potentially causing a change in variables like ConnectionType while Update() is still running.

You can keep multiple threads from mutating ConnectionType and ReturnValue with the Monitor/Lock construct. That should be relatively straightforward.

2) Deal with Refresh / Unload in a graceful way.

No idea. Perhaps setting the thread to t.isBackground=true would help there? In that case the child thread stops abruptly when the parent thread is terminated.

[Aragas]

About separating. Store all option in one value can cause potentially errors. Rainmeter calls Reload() and Update() separate, so ConnectionType and ReturnValue is't synchronised.
A sample:
We need to get Internet status. Rainmeter calls Reload(), where we get ConnectionType. Then he calls Update(). Plugin read ConnectionType and starts the Thread. And, because we can't wait the Thread ends and give us the value, he will give back Rainmeter 0.0 or something like that. We will probably get the value when Rainmeter secondly calls Update(). 20 second haven't passed, so we will not make a new Thread. We just give Rainmeter the value. But! We could make before the second Update() a new Reload() call, where we want get the Network status. We make a new Thread. The second Update() can give us the Network status, not the Internet.
By separating the values we can pass that error. but this only works if i have right understand that Rainmeter can call whenever Reload().