Make query async

Question

I am making a simple login form application that is connected with my MySQL database.

Here is the code that I using for verifying login credentials.

 private void login()
        {
            try
            {
                MySqlConnection connection = new MySqlConnection("server=sql186.main-hosting.eu; userid=u946814739_SQL; password=testing; database=u946814739_MySQL;");
                MySqlDataReader reader;
                MySqlCommand cmd = new MySqlCommand("Select * FROM users WHERE username='" + textBox1.Text + "' and password='" + textBox2.Text + "'    ", connection);
                connection.Open();
                reader = cmd.ExecuteReader();
                int count = 0;
                while (reader.Read())
                {
                    count = count + 1;
                }
                if (count == 1)
                {
                    //login successful
                    MessageBox.Show("Login success");
                }
                else
                {
                    MessageBox.Show("Login failed");
                }
                connection.Close();
                connection.Dispose();
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

It all works great, but now I want to know how to make this method async. So my application doesn't freeze while it's retrieving the data. Any help would be appreciated.

Thanks

1. Make routine `async` 2. Wrap the code in a `await Task.Run` 3. Change code to use the `async` methods for `MySQL`. 4. Enjoy — Trevor, Sep 23 '19 at 19:36
Please learn how to ***parameterize your queries***. This is ripe for SQL injection. And then use `using` blocks to ensure your disposables are disposed properly. — madreflection, Sep 23 '19 at 19:44
Don't use string concatenation either, use parameters. Put this in `textBox2.Text` for example: `'; DROP TABLE users; --` — Trevor, Sep 23 '19 at 19:45
@Çöđěxěŕ Just wrapping code in a `Task.Run` is not a great approach. All that does is push this IO bound code into another thread that then gets blocked. It's much better to use built in async methods so that no threads are blocked during the IO. — juharr, Sep 23 '19 at 20:15
@juharr agreed, my point is change the methods to async in which MySQL provides and or wrap the code itself; maybe I should have re-worded that. Either way it's still going to block with a `MessageBox` call..., pick your poison. TBH, it should just be a function. — Trevor, Sep 23 '19 at 20:17
If you want to use async DB methods with MySQL, you need a MySQL ADO.NET library that implements async correctly. Oracle's MySql.Data doesn't (it will just run all the "Async" methods synchronously), so upgrade to https://www.nuget.org/packages/MySqlConnector/ to get true async support. — Bradley Grainger, Sep 24 '19 at 13:27

score -1 · Answer 1 · answered Sep 23 '19 at 19:36

-1

you could do this w/ async await pattern

you also don't need to use a reader to read each row to determine how many matches you got. you can simply use Count(*)

By using using statement you can get that .Dispose (dispose calls close) for free inside a finally statement to make sure the connection is closed even if theres an exception during the execution.

        async void login()
        {
            try
            {
                using (MySqlConnection connection = new MySqlConnection("-"))
                {
                    //1st dont store password in plain text
                    //2nd dont use string concat. google sql injection plz
                    using (MySqlCommand cmd = new MySqlCommand("Select COUNT(*) FROM users WHERE username='" + textBox1.Text + "' and password='" + textBox2.Text + "'    ", connection))
                    {
                        connection.Open();

                        int count = (int)await cmd.ExecuteScalarAsync();
                        if (count == 1)
                        {
                            //login successful
                            MessageBox.Show("Login success");
                        }
                        else
                        {
                            MessageBox.Show("Login failed");
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }

answered Sep 23 '19 at 19:36

Steve

11,696
7
43
81

Using your code. Getting an error "Specified cast is not valid" (wasn't me who downvoted, just gonna throw that in there) – Jahanzaib Bokhari Sep 23 '19 at 19:50
@JahanzaibBokhari weird Mysql. maybe it returned long or double. Just check the return type and cast it accordingly – Steve Sep 23 '19 at 19:53
@JahanzaibBokhari what if `null` is returned, the cast will fail; did you check this? See this https://dev.mysql.com/doc/dev/connector-net/8.0/html/M_MySql_Data_MySqlClient_MySqlHelper_ExecuteScalarAsync.htm specifically the `Return Value` section. – Trevor Sep 23 '19 at 19:55
1

@Çöđěxěŕ count will never return null. at least not in Sql Server. not a Mysql guy but i assume it should be the same. – Steve Sep 23 '19 at 20:07
`cmd.ExecuteScalarAsync()` *can return null* and you are casting it; it *can* fail if it doesn't get a result set... – Trevor Sep 23 '19 at 20:09
@Çöđěxěŕ The method can return `DBNull` depending on the query, but `Count` will never return null. Either the query results in 0 or more rows so `Count` will always be 0 or higher. – juharr Sep 23 '19 at 20:10
@juharr/@steve who said count would be null, I said above `what if null is returned` meaning from the `await cmd.ExecuteScalarAsync()` call. – Trevor Sep 23 '19 at 20:12
@Çöđěxěŕ The query is using `Count`. Thus executing the query will never return null. – juharr Sep 23 '19 at 20:12
@juharr what if user typed `'Obrien` as the password for textBox2; that will just throw an exception then right? – Trevor Sep 23 '19 at 20:22
1

So I casted it as long and it works but the function is not async, the application is freezing still. Even used: await connection.OpenAsync(); But still freezing. – Jahanzaib Bokhari Sep 23 '19 at 20:30
@JahanzaibBokhari thats because MessageBox.Show is a blocking call. Your DB portion is async but MessageBox is not. you could either show the msgbox in a different thread or make your own msgbox window and call .Show instead of .ShowDialog – Steve Sep 23 '19 at 20:40
i have removed all the message box but still same thing. – Jahanzaib Bokhari Sep 23 '19 at 20:59
@JahanzaibBokhari how are you calling the login method? I would assume the query wouldnt take less than a split of a second to complete. so it shouldnt freeze even without async. – Steve Sep 23 '19 at 21:04
@Çöđěxěŕ Yes, a sql injection could cause it to do all sorts of bad things like throw an exception or even return null, but assuming the data is correct it should return a number. – juharr Sep 23 '19 at 21:17
1

@JahanzaibBokhari You really should make this `async Task` and then call it with `await login` then you'll have to let the async/await pattern bubble up the call stack to the top. Or really `async Task` so you can return if it succeeds or not. – juharr Sep 23 '19 at 21:18
Don’t do `async void`. Except if it’s an event handler and there only because event handlers can only return void. – ckuri Sep 23 '19 at 22:07
Alright, I will be trying that next. Thanks for help everyone! – Jahanzaib Bokhari Sep 23 '19 at 23:51
1

MySql.Data doesn't implement async methods correctly; see https://mysqlconnector.net/tutorials/migrating-from-connector-net/#async The fix is to upgrade to https://www.nuget.org/packages/MySqlConnector/ an OSS MySQL library for .NET that implements async correctly. – Bradley Grainger Sep 24 '19 at 13:26

Make query async

1 Answers1