Reputation: 7809
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
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
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
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