Using String.Empty as a SyncLock object

[ilitirit]

Are there any reasons against writing code like this?

class Foo
{
static string _SyncLock = String.Empty;

public void ThreadSafeMethod()
{
lock (_SyncLock)
{
// Code goes here
}
}
}

 

[Andy]

Are there any reasons against writing code like this?

class Foo
{
static string _SyncLock = String.Empty;

public void ThreadSafeMethod()
{
lock (_SyncLock)
{
// Code goes here
}
}

}

I don't see anything wrong with it; you're locking on an instance,
what it contains is irrelevant. An object may have less overhead
though, because it doesn't need a character array..

 

[Andy]

Are there any reasons against writing code like this?

class Foo
{
static string _SyncLock = String.Empty;

public void ThreadSafeMethod()
{
lock (_SyncLock)
{
// Code goes here
}
}

}

My only concern would be that string.Empty may be a singleton, and if
other code decides to lock on it, you could head a deadlock
condition. Might be better just to lock on an object.

 

[Brian Gideon]

Are there any reasons against writing code like this?

class Foo
{
static string _SyncLock = String.Empty;

public void ThreadSafeMethod()
{
lock (_SyncLock)
{
// Code goes here
}
}



}

Hi,

I'm fairly confident that String.Empty returns the same instance
everytime. That means if you're using this strategy consistently
throughout your application you'll have completely independent
sections of code competing for the same lock. Why not instantiate an
Object for this purpose?

Brian

 

[ilitirit]

Thanks guys. I had the feeling this was the case but I just wanted to
make sure. This code is part of a .NET 1.1 application that I'm
supposed to migrate so I guess I might as well start fixing things
like this as well.

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

Are there any reasons against writing code like this?

class Foo
{
static string _SyncLock = String.Empty;

public void ThreadSafeMethod()
{
lock (_SyncLock)
{
// Code goes here
}
}
}

As already have been said, String.Empty always is the same instance, so
locking on that means that all locks block each other even if they are
completely independend. Use new Object() to create a unique object for
the lock.

When locking in a static method, use a static variable for locking, but
when locking in an instance method, use an instance variable for locking.

 

[Fabio]

"Göran Andersson" <[email protected]> ha scritto nel messaggio

As already have been said, String.Empty always is the same instance, so
locking on that means that all locks block each other even if they are
completely independend.

When you do

static string _SyncLock = String.Empty;

You create a new istance, so the problem isn't the string.Empty, but the
"static" of _SyncLock.
Of course, a

object _SyncLock = new object();

would be more clear.

 

[Mike Schilling]

"Göran Andersson" <[email protected]> ha scritto nel messaggio



When you do

static string _SyncLock = String.Empty;

You create a new istance, so the problem isn't the string.Empty, but
the "static" of _SyncLock.

Why do you say that? "=" doesn't create new instances, it assigns
references.

 

[Fabio]

Why do you say that? "=" doesn't create new instances, it assigns
references.

It's a string: it's immutable.

 

[Willy Denoyette [MVP]]

It's a string: it's immutable.

Sure, but that's not the point, you are setting a reference to an existing instance. More,
as others have said, there is only one instance of String.Empty per process, that means that
each reference to String.Empty will point to the same instance.

Willy.

 

[Zytan]

It's a string: it's immutable.

Sure, but that's not the point, you are setting a reference to an existing instance. More,
as others have said, there is only one instance of String.Empty per process, that means that
each reference to String.Empty will point to the same instance.

Exactly. MSDN is actually very clear on this point:

http://msdn2.microsoft.com/en-us/library/c5kehkcz.aspx
"lock("myLock") is a problem since any other code in the process using
the same string, will share the same lock."

Zytan

 

[Jon Skeet [C# MVP]]

Are there any reasons against writing code like this?

Absolutely - *anything* else that decides to do the same thing will be
using the same lock, even though you really don't want to! It's just
*waiting* for a deadlock...

Why use that rather than just a plain new object?

 

[Jon Skeet [C# MVP]]

It's a string: it's immutable.

That's irrelevant - or in fact, it's the only reason that string.Empty
*can* legitimately return a reference to the same object every time).
Try this:

using System;

class Test
{
static void Main()
{
string x = string.Empty;
string y = string.Empty;

Console.WriteLine (object.ReferenceEquals(x,y));
}
}