Athiwat Chunlakhan
Athiwat Chunlakhan

Reputation: 7809

Accessing the same string(StringBuilder) using multi-thread

My problem is if I use multi-thread on the same string sometime

the string won't get replace.(I wrote this on notepad so syntax may be

wrong)

using System.Thread ... Others ofcourse

class ....
{
    private static StringBuild container = new StringBuilder();

    static void Main(...)
    {
    container.Append(Read From File(Kind of long));
    Thread thread1 = new Thread(Function1);
        Thread thread2 = new Thread(Function2);
    thread1.Start();
    thread2.Start();
    //Print out container
    }

    static void Function1
    {
    //Do calculation and stuff to get the Array for the foreach
    foreach (.......Long loop........)
    {
    container.Replace("this", "With this")
    }
    }
    //Same goes for function but replacing different things.
    static void Function2
    {
    //Do calculation and stuff to get the Array for the foreach
    foreach (.......Long loop........)
    {
    container.Replace("this", "With this")
    }
    }
}

Now sometime some element does not get replace. So my solution to this is calling container.Replace on a different

method and do a "lock" which work but is it the right way?

private class ModiflyString
{
        public void Do(string x, string y)
            {
                lock (this)
                {
                    fileInput.Replace(x, y);
                }
            }
}

Upvotes: 3

Views: 8010

Answers (3)

AaronLS
AaronLS

Reputation: 38394

That will work fine as long as both threads have the same instance of the ModifyString class. In other words, this will work because the lock on "this" must be a lock on the same instance:

class Blah
{
    private static StringBuild container = new StringBuilder();

    private static ModifyString modifyString = new ModifyString();

    static void Main(...)
    {
    container.Append(Read From File(Kind of long));
    Thread thread1 = new Thread(Function1);
        Thread thread2 = new Thread(Function2);
    thread1.Start();
    thread2.Start();
    //Print out container
    }

    static void Function1
    {       

        //Do calculation and stuff to get the Array for the foreach
        foreach (.......Long loop........)
        {
           modifyString.Do("this", "With this")
       }
    }
    //Same goes for function but replacing different things.
    static void Function2
    {
        //Do calculation and stuff to get the Array for the foreach
        foreach (.......Long loop........)
        {
            modifyString.Do("this", "With this")
        }
    }
}

It will NOT work if you did the below, because lock(this) would not work sense they are two seperate instances:

class Blah
{
    private static StringBuild container = new StringBuilder();

    static void Main(...)
    {
    container.Append(Read From File(Kind of long));
    Thread thread1 = new Thread(Function1);
        Thread thread2 = new Thread(Function2);
    thread1.Start();
    thread2.Start();
    //Print out container
    }

    static void Function1
    {
       ModifyString modifyString = new ModifyString();
       //Do calculation and stuff to get the Array for the foreach
       foreach (.......Long loop........)
       {
          modifyString.Do("this", "With this")
       }
    }
    //Same goes for function but replacing different things.
    static void Function2
    {
       ModifyString modifyString = new ModifyString();

       //Do calculation and stuff to get the Array for the foreach
        foreach (.......Long loop........)
        {
            modifyString.Do("this", "With this")
        }
    }
}

Some people would actually create a "dummy" object to perform the lock on instead of using "this" (you can't lock on the string since it is a value type).

Upvotes: 2

Henk Holterman
Henk Holterman

Reputation: 273591

Your locking won't work when you create more than 1 ModifyString object and I'm guessing you do.

A simple version:

   public void Do(string x, string y)
   {
      lock (fileInput)
      {
         fileInput.Replace(x, y);
      }
   }

It may be better to create a separate object to do the locking on, but the above shows the principle better: all competing threads should lock on the same object.

A standard approach would look like:

private static StringBuild container = new StringBuilder();
private static object syncLock = new object();  // simple object, 1-1 with container

and then you can (thread-)safely use:

   lock(syncLock)
   {
       container.Replace(...);
   }

Upvotes: 3

Philippe Leybaert
Philippe Leybaert

Reputation: 171864

You should lock the StringBuilder object itself (inside the replace functions):

lock (container)
{
   container.Replace("this", "With this");
}

or create a separate lock object:

static object _stringLock = new object();

...

lock(stringLock)
{
    container.Replace("this", "With this");
}

Upvotes: 6

Related Questions