Threading related naming and usage of const

HobbyProggy

Hey guys, i was facing an error on my logging today which made me struggle a bit. To clarify the issue i wanted to know if anyone knows such things happen on a regular basis or is just bad luck. We are within a REST Endpoint where multiple users fire requests to, the Endpoint will then process all these requests async and return the results. There problem happened here (i narrowed it down to the important parts and underlined them):

private ProjectInformationBase? GetProjectBaseByDevelopmentProject(Guid idProject)
{
const string getProject = "SELECT * FROM Projects WHERE idProject = @idProject";

        var getProjectCommand = new SqlCommand(getProject, DbConnection);
        getProjectCommand.Parameters.Add("@idProject", SqlDbType.UniqueIdentifier).Value = idProject;

        var projectData = getProjectCommand.ReadDataByCommand();

        if (projectData.Rows.Count == 1)
        {
            return new ProjectInformationBase
            {
                **IdProjectInformationBase = (Guid)projectData.Rows\[0\]\["idProject"\]**, //this one crashed
            };
        }

        return null;
    }

    private ProjectInformationBase? GetProjectBaseByEcrProject(Guid idProject)
    {
        const string getEcrProject = "SELECT \* FROM EcrProject WHERE ecrIdProject = @idProject";

        var getEcrProjectCommand = new SqlCommand(getEcrProject, DbConnection);
        getEcrProjectCommand.Parameters.Add("@ecrIdProject", SqlDbType.UniqueIdentifier).Value = idProject;

        var projectData = getEcrProjectCommand.ReadDataByCommand();

        if (projectData.Rows.Count == 1)
        {
            try
            {
                return new ProjectInformationBase
                {
                    IdProjectInformationBase = (Guid)projectData.Rows\[0\]\["ecrIdProject"\],
                };
            }
            catch (Exception e)
            {
                throw;
            }
        }

        return null;
    }

On the log it said the column "idProject" wasn't found, which looks to me like it was using the wrong method / sql request or dataTable "projectData". Honestly i thought since they get created and populated each time they should be "threadsafe" and not interfere with each other. Anyone care to explain what could have hap

Richard Deeming

Where is getProjectCommand defined, and what does the ReadDataByCommand method look like? Is there a reason you're using SELECT * FROM ... instead of specifying the list of columns you want to load? That usually leads to performance problems, where you're loading more data that you actually need.

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

Lost User

OK but can you expand on what you mean by "threading related naming and usage of const" though? The `var projectData` in each method are completely unrelated by the way. They're different variables, having the same name doesn't have any meaning except to humans who read the source code.

HobbyProggy

The Command is defined in that "..." area, i just didn't want to bloat all the code inside. ReadDataByCommand is a simple extension:

public static DataTable ReadDataByCommand(this SqlCommand command, bool mapResultsToDatabase = true)
{
try
{
using (var sqlDataAdapter = new SqlDataAdapter(command))
{
var resultTable = new DataTable();

                if (mapResultsToDatabase)
                {
                    sqlDataAdapter.FillSchema(resultTable, SchemaType.Mapped);
                }

                sqlDataAdapter.Fill(resultTable);

                return resultTable;
            }
        }
        catch (SqlException ex)
        {
            var exNew = new Exception(command.BuildErrorMessage(), ex);
            throw exNew;
        }
        catch (InvalidOperationException ex)
        {
            var exNew = new Exception(command.BuildErrorMessage(), ex);
            throw exNew;
        }
        catch (Exception ex)
        {
            var exNew = new Exception(command.BuildErrorMessage(), ex);
            throw exNew;
        }
    }

I am loading all columns because i'll need all in the future, for testing purposes we just read out one yet.

Rules for the FOSW ![^]

MessageBox.Show(!string.IsNullOrWhiteSpace(_signature)
? $"This is my signature:{Environment.NewLine}{_signature}": "404-Signature not found");

HobbyProggy

I'll update that in the initial post. I would assume so too, because everything is only declared and instantiated in each method separately. Nonetheless I had this strange behavior.

Rules for the FOSW ![^]

MessageBox.Show(!string.IsNullOrWhiteSpace(_signature)
? $"This is my signature:{Environment.NewLine}{_signature}": "404-Signature not found");

Richard Deeming

Nothing obviously wrong there. However:

Quote:

Please note that the "DbConnection" is defined within the class and the class is registered as Singleton.

That's a really bad idea. Every request will be fighting to use the same connection instance. Instead, create the connection when you need it, and wrap it in a using block to ensure it's always disposed of properly when you've finished with it. Connection pooling[^] will automatically take care of keeping a pool of database connections hanging around in case you're worried about the cost of creating the connection.

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

HobbyProggy

I do know and found that out earlier, i think this is actually the only place where those two methods will do this. So you think it could be related to that? Because I had worse issues in the past and then decided that each time a request to the SQL Server gets fired we will create a new connection, with the exception of queries that need to be done one after another in a single method.

Rules for the FOSW ![^]

MessageBox.Show(!string.IsNullOrWhiteSpace(_signature)
? $"This is my signature:{Environment.NewLine}{_signature}": "404-Signature not found");

Richard Deeming

The SqlConnection class isn't thread-safe. If you're storing a single connection instance in a field of a singleton class, and then using that for queries across multiple threads, I wouldn't be surprised if the queries ended up with some cross-contamination. :)

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

HobbyProggy

After reading that, i must admit, yes. I have no other words to describe my feelings right now :~

Rules for the FOSW ![^]

MessageBox.Show(!string.IsNullOrWhiteSpace(_signature)
? $"This is my signature:{Environment.NewLine}{_signature}": "404-Signature not found");

Lost User

I think the usual convention is to "capitalize" column names in the database; to avoid the confusion you're experiencing with similar looking "variables". I would think the "data base column definitions" should be looked at. Maybe somebody changed them when you weren't looking. There's nothing to indicate you're actually accessing the correct database or table (connection strings).

"Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I

jschell

Expanding a bit more on why more than one DBConnection is needed. This is not just a feature of the class. Rather it is the nature of the connection to the database itself. For example most if not all databases have an explicit 'transaction' (in some definition of that word) when a statement runs across the connection. Regardless of whether the transaction type is changed by the code. And this is tied to the connection itself. So for example a long running query (seconds hopefully and not minutes) has transaction type X. Then you attempt to use it in another process thread where you want to change the transaction type to Y. And then you send that second statement down the same connection. Which transaction is then in play. Why do they do it that way? Because if the connection is lost the database must decide whether the currently running statement succeeded or failed. And different database DO decide that differently. Some decide it success which means they might commit a transaction. Others deem it a failure, so the fail the transaction.

HobbyProggy

Yep, you are absolutely right. I already was able to "reconstruct" that behavior and it is simply the lack of connections established that leads to this problem. So basically each time i need to do a query which is not part of a greater transaction we open up another connection. Issue solved :)

Rules for the FOSW ![^]

MessageBox.Show(!string.IsNullOrWhiteSpace(_signature)
? $"This is my signature:{Environment.NewLine}{_signature}": "404-Signature not found");