views:

238

answers:

2

Hi, 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);
                }
            }
}
+3  A: 

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");
}
Philippe Leybaert
I never thought of locking the stringbuilder itself, very nice. thanks,
Jonathan Shepherd
Phil's second example is what I'm referring to when I mentioned a "dummy" object. However, I think his first example where he locks the container is the best of them all. Since the container represents the data that is being shared between threads, you want to lock on that. If you used my example, there is the problem that if you had threads which are trying to call ModifyString.Do for totally separate containers, they will block one another, even though the containers they are working with are totally different. So you would hurt performance. So my example is really a bad implementation.
AaronLS
thanks @arronls now my program is working :)
Jonathan Shepherd
I can't edit this, anyway what is better locking on the object itself or locking on a "dummy" object
Jonathan Shepherd
+2  A: 

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).

AaronLS
StringBuilder is not a value type you know that right?
Jonathan Shepherd
I didn't say stringbuiler, I said string.
AaronLS