2

So basically I'm using C# to write a port scanning WPF application, that will scan over a thousand combinations of ports and IPs (x.x.x.x:xx). As a result, I have to split this process up into multiple threads to quicken the process and avoid making the UI freeze. Shown below is my code for the MainWindow:

namespace PortScannerTool
{
    public partial class MainWindow : Window
    {

        public MainWindow()
        {
            InitializeComponent();
            tb_portstatus.Text = "Click the button to query ports";
            dns_info.Text = tb_portstatus.Text;
        }


        private void Btn1_Click(object sender, RoutedEventArgs e)
        {

            CmdExec cmdobj = new CmdExec("127.0.0.0","8080");
            Thread query_status = new Thread(cmdobj.Runcmd);
            query_status.Start();
            tb_portstatus.Text = "Done!";
        }
    }
}
namespace PortQueryApp
{
    class CmdExec
    {

        string command;
        public string dnsText;
        public string querystatusText;

        public CmdExec(string ip, string port)
        {
           this.command = $"-n {ip} -o {port}";
        }

        public void Runcmd()
        {
            Process proc = new Process();
            proc.StartInfo.FileName = "PortQry.exe";
            proc.StartInfo.Arguments = this.command;
            proc.StartInfo.CreateNoWindow = true;
            proc.StartInfo.UseShellExecute = false;
            proc.StartInfo.RedirectStandardOutput = true;
            proc.Start();
            string query_output = proc.StandardOutput.ReadToEnd();
            proc.WaitForExit();

            string resolve_pass = "Name resolved";
            string resolve_fail = "Failed to resolve IP address to name";

            if (query_output.Contains(resolve_pass))
            {
                this.dnsText = "Resolved!";
            }
            if (query_output.Contains(resolve_fail))
            {
                this.dnsText = "Resolve Failed!";
            }

            this.querystatusText = "Done!";

        }
    }
}

dns_info and tb_portstatus are XAML Textboxes. CmdExec.Rumcmd is the method that does the actual port scanning itself. If you look at the CmdExec class, PortQry.exe is the microsoft port query tool.

So I want to get the cmdobj.Runcmd method to update the UI once it's done running. But I don't know how to do so, I've Googled this extensively, and I'm getting some people suggesting using delegates, Tasks and BackgroundWorkers, not sure which method would be best for my case.

Could anyone give some suggestions or point me to any guides? That'd be awesome! Thanks!

Edit: Most examples have their Multithreaded method within the same class as well. I'm not sure if this is required, but should I use:

 class CmdExec : MainWindow
{
   //Do stuff
}

instead?

John
  • 155
  • 8
  • What's happening in `cmdobj.Runcmd`? Look for async methods. – Jeroen van Langen May 31 '19 at 08:12
  • 1
    FYI [Tasks are not threads and async isn't parallel](https://blogs.msdn.microsoft.com/benwilli/2015/09/10/tasks-are-still-not-threads-and-async-is-not-parallel/). And if you're looking to update the gui from another place than the main thread using a [callback delegate](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/delegates/index) is in order – MindSwipe May 31 '19 at 08:16
  • @MindSwipe Are there any posts that explain how to use one? – John May 31 '19 at 08:24
  • If you post an [mcve] of the CmdExec class I could create an example out of it (please keep in mind to keep the example minimal) – MindSwipe May 31 '19 at 08:27
  • @MindSwipe Updated, the class just has one method that calls an exe. – John May 31 '19 at 08:35

3 Answers3

2

What you SHOULD do is: Give some events CmdExec so that they can be fired when something changes, for example:

public delegate void CombinationFoundDelegate(object sender, int port, string ip); 
//note that it would be better to create class derived from EventArgs instead of passing bare arguments in this delegate
public class CmdExec
{
    public event CombinationFoundDelegate CombinationFound;

    public void Runcmd()
    {
       //do your work and call event when necessary:
       if(CanConnect(ip, port))
         CombinationFound?.Invoke(this, port, ip);
    }
}

And then on your form just listen to this event:

cmdobj.CombinationFound += new CombinationFoundCallback;

And all the GUI updates may happen in this callback. But, you have to be aware that this callback is called from different thread (as CmdExec runs on different thread), so you need to synchronize your controls. You can use it Dispatcher for that case:

Action a = new Action(() => textBox.Text = ip); 

if(!Dispatcher.CheckAccess())
   Dispatcher.Invoke(a);
else
   a();

So, note that CheckAccess is for some reason hidden in Intellisense, but it's really there. CheckAccess just cheks wheter synchronization is required or not (in Winforms it would be InvokeRequired).

Next, if synchronization is required, you can run your UI update code inside dispatcher invoke method. However, if not synchronization is required, you can just run your ui code here. That's why I have created Action for this - not to duplicate the code.

And that's simply it. (Code written from head, so there can be some difference with real code, but this is how it should look and work).

Adam Jachocki
  • 1,897
  • 1
  • 12
  • 28
  • Hi, thanks for the very detailed answer! One issue though, my plan is to generate a CmdExec object for each group of ports(Grouped by a particular service), I'll be reading in an initial file and will dynamically create CmdExec objects. Seeing as your delegate was defined outside of the CmdExec class, this might be an issue, as I'll to dynamically create delegates as well. – John May 31 '19 at 09:39
  • Event was defined insinde CmdExec. But it has been handled by the form. You can do what you want with that. You can add for example another parameter that would give a group or something. Anyway CmdExec really should not touch any GUI by itself. – Adam Jachocki Jun 01 '19 at 16:28
0

I think BackgroundWorker is better choice. In WPF, you are not allowed to make changes to the UI from another thread. So, simply moving code in the event handler method causes a problem when reflecting process result on UI (e.g. showing message "done").

It is still the same in BackgroundWorker.DoWork, but BackgroundWorker's ReportProgress and RunWorkerCompleted can help you to have your code simple. Have a look at the answer here. I think you can get the point easily.

Yas Ikeda
  • 973
  • 1
  • 9
  • 16
0

You should use Threadless async (at least no managed threads).

You can await the work done the process with a custom awaiter. The work to do this is pretty involved. Luckily someone has already done this.

PM> Install-Package RunProcessAsTask

Then your code just becomes

    public async Task<bool> Runcmd()
    {
        ProcessResults results = await ProcessEx.RunAsync(
            "PortQry.exe",
             command
        );
        if(results.StandardOutput.Any(output => output.Contains(resolve_pass)))
           return true;
        return false;
    }
Aron
  • 15,464
  • 3
  • 31
  • 64