Which design pattern violation does this bug do and what to call it?

Question

There is a singleton service class ItemsDataSource: IItemsDataSource injected into many other classes (business domain classes)

These business domain classes are many and run asynchronously calling methods on that ItemsDataSource service.

public interface IItemsDataSource
{
    Task<IEnumerable<string>> GetItemsAsync();
    void SetSourceConfiguration(JToken src);
}

public class ItemsDataSource : IItemsDataSource
{
    private JToken m_configuration;
    public Task<IEnumerable<string>> GetItemsAsync()
    {
        // Use m_configuration to some items
    }
    public void SetSourceConfiguration(JToken config)
    {
        m_configuration = src;
    }
}

When multiple classes that are using this service are running asynchronously (let's say on 2 threads T1 and T2), this is sometimes happening:

T1 calls SetSourceConfiguration(config1) then starts running GetItemsAsync() asynchronously. T2 calls SetSourceConfiguration(config2) (m_configuration is now assigned with config2) before T1 is done running GetItemsAsync(). For that T1 uses config2 instead of config1 and unexpected behavior happens.

The questions:

1- The optimal fix I think is removing SetSourceConfiguration and passing the JToken config directly as parameter into GetItemsAsync, or locking the code in the business classes, or is there another better solution ?

2- Which design pattern violation caused this bug ? So It could be avoided in the first place.

3- What is the "technical" term for this bug ? Methods with Side Effects, Design pattern violation, etc. ?

Re, "Which design pattern violation caused this bug ? So It could be avoided in the first place." You aren't going to write bug-free code by paying attention to "design patterns." You have to pay attention to your own code. You have to think carefully about what it does. (E.g., what does it mean when two threads each want to set the same global variable to a different value.) Design patterns are just that: patterns that programmers tend to use again and again. Giving them names makes it easier for us to explain how our own code works when we happen to use one. — Solomon Slow, Sep 30 '22 at 15:13

StepUp · Answer 1 · 2022-09-30T11:21:03.910

1- The optimal fix I think is removing SetSourceConfiguration and passing the JToken config directly as parameter into GetItemsAsync, or locking the code in the business classes, or is there another better solution ?

If your service ItemsDataSource is not singleton service, you can remove SetSourceConfiguration and passing the JToken config directly as parameter into GetItemsAsync. However, it is singleton service, so the way to go, imho, is using lock of critical section.

The code would look like this, if you are using C#.

private readonly object myLock = new object();

public void Foo()
{
    lock (myLock )
    {
        // critical section of code here
    }

}

Which design pattern violation does this bug do and what to call it?

1 Answers1