Задачка на многопоточность (.NET)?

Question

[andreycha]

Какая проблема есть в коде? Как изменить код так, чтобы ее избежать?

using System;
using System.Threading;

namespace Mover
{
    internal class Endpoint
    {
        public int Amount { get; set; }
    }

    internal class Program
    {
        private static void Main(string[] args)
        {
            var source = new Endpoint();
            var target = new Endpoint();

            var initialAmount = 1000000;
            source.Amount = initialAmount;

            var thread = new Thread(new ThreadStart(delegate
                                                    {
                                                        Transfer(source, target, initialAmount);
                                                    }));
            thread.Start();
            Transfer(target, source, initialAmount / 2);
            thread.Join();

            Console.Out.WriteLine("source.Amount = {0}", source.Amount);
            Console.Out.WriteLine("target.Amount = {0}", target.Amount);
        }

        private static void Transfer(Endpoint source, Endpoint target, int count)
        {
            while (count-- > 0)
                lock (target)
                    lock (source)
                    {
                        source.Amount--;
                        target.Amount++;
                    }
        }
    }
}

Answer 1

[VenomBlood]

Предложу своё решение:

    public class Endpoint
    {
        private Mutex _sendLock;
        private Mutex _receiveLock;

        private int _amount;
        public int Amount
        {
            get
            {
                return _amount;
            }
        }

        public Endpoint(int amount)
        {
            _sendLock = new Mutex();
            _receiveLock = new Mutex();

            _amount = amount;
        }

        public static void Transmit(Endpoint source, Endpoint destination, int amount)
        {
            WaitHandle[] transmittingPartiesHandles = new WaitHandle[] { source._sendLock, destination._receiveLock };
            WaitHandle.WaitAll(transmittingPartiesHandles);

            // Do smth.

            Interlocked.Decrement(ref source._amount);
            Interlocked.Increment(ref destination._amount);

            source._sendLock.ReleaseMutex();
            destination._receiveLock.ReleaseMutex();
        }
    }

public class Program
    {
        public static void Main(string[] args)
        {
            Endpoint detroit = new Endpoint(1000000);
            Endpoint london = new Endpoint(0);

            int amount = detroit.Amount;

            Thread thread = new Thread(() => Transfer(detroit, london, amount));
            
            thread.Start();
            Transfer(london, detroit, amount / 2);
            thread.Join();

            Console.Out.WriteLine("source.Amount = {0}", detroit.Amount);
            Console.Out.WriteLine("target.Amount = {0}", london.Amount);
            Console.ReadLine();
        }

        private static void Transfer(Endpoint source, Endpoint target, int amount)
        {
            for (int i = 0; i < amount; i++)
            {
                Endpoint.Transmit(source, target, amount);
            }
        }
    }

Во первых — делаем объект Endpoint потокобезопасным сам по себе. Ведь по сути изменять свойство Amount из внешнего кода нельзя.
Во вторых — избавляемся от локов на реальные объекты (это плохо, т.к. может существовать параллельно другой сценарий, в котором для совершенно других целей опять возьмётся лок на тот же объект Endpoint)
В третьих — выделяем процесс передачи в отдельный метод. Общий лок здесь не пройдет если будет настроенно сразу несколько передач в разных местах (будет много объектов «общей» блокировки для одной и той же пары конечных точек, из за чего возникнут проблемы).