What did he want to lock?
-
Maybe he was afraid that someone/something (like Windows, CLR or even God) would change his path containing local string before he got to delete the file. :) Does that count as paranoia? :-D
I have no smart signature yet...
-
I have no idea what he locks in this C# code.
// path is a local variable.
lock (path)
{
if (File.Exists(path))
{
File.Delete(path);
}
}:doh:
-
I have no idea what he locks in this C# code.
// path is a local variable.
lock (path)
{
if (File.Exists(path))
{
File.Delete(path);
}
}It is possible, I suppose, that path was originally an instance variable and this was subsequently refactored to make it local to the method. That would explain it, if someone just didn't bother removing the lock on path when they did the refactoring. It's also possible, of course, that whoever wrote it just didn't understand lock.
-
I have no idea what he locks in this C# code.
// path is a local variable.
lock (path)
{
if (File.Exists(path))
{
File.Delete(path);
}
}To add to David's reply, the intention might have been to protect against two threads of the same program concurrently trying to delete the file (would work if both threads lock on the smae path instance). Or maybe a generous hope that this gives him a somehow-transitional file system.
Agh! Reality! My Archnemesis![^]
| FoldWithUs! | sighist | µLaunch - program launcher for server core and hyper-v server. -
It is possible, I suppose, that path was originally an instance variable and this was subsequently refactored to make it local to the method. That would explain it, if someone just didn't bother removing the lock on path when they did the refactoring. It's also possible, of course, that whoever wrote it just didn't understand lock.
lockdoesn't care whether it's an instance or local variable. It's locking the object, not the variable. As strings with the same content might or might not be shared by the .NET runtime (depending on how the strings were created, .NET version, compilation options, etc.), no one can ever know what exactly that code was locking. But even if that originally was an instance variable, the original code would have been incorrect.David Skelly wrote:
It's also possible, of course, that whoever wrote it just didn't understand lock.
That's the only possible explanation.
-
lockdoesn't care whether it's an instance or local variable. It's locking the object, not the variable. As strings with the same content might or might not be shared by the .NET runtime (depending on how the strings were created, .NET version, compilation options, etc.), no one can ever know what exactly that code was locking. But even if that originally was an instance variable, the original code would have been incorrect.David Skelly wrote:
It's also possible, of course, that whoever wrote it just didn't understand lock.
That's the only possible explanation.
Daniel Grunwald wrote:
That's the only possible explanation.
I'm amazed at how many people seem to think that the way to avoid problems with concurrent access to an object is to arbitrarily pick something kinda-sorta related to that object and lock it. They see all the warnings about the risks of locking publicly-available objects and try to avoid doing that, despite the fact that if a lock must be held between calls to an objects methods or properties, it must be publicly available. To be sure, it's often best if things can be arranged so a lock needn't/won't be held while running 'outside' code, but when it's necessary one shouldn't try to avoid exposing the lock.
-
Daniel Grunwald wrote:
That's the only possible explanation.
I'm amazed at how many people seem to think that the way to avoid problems with concurrent access to an object is to arbitrarily pick something kinda-sorta related to that object and lock it. They see all the warnings about the risks of locking publicly-available objects and try to avoid doing that, despite the fact that if a lock must be held between calls to an objects methods or properties, it must be publicly available. To be sure, it's often best if things can be arranged so a lock needn't/won't be held while running 'outside' code, but when it's necessary one shouldn't try to avoid exposing the lock.
supercat9 wrote:
if a lock must be held between calls to an objects methods or properties, it must be publicly available.
That isn't quite true. An instance member marked
privatecan be the lock value, manipulated solely by members of the class, like this:class ThreadedWhatsit
{
public ThreadedWhatsit
{
lock(_Lock)
{
// do stuff
}
}
public ThingType PropertyA
{
set
{
lock(_Lock)
{
_PropertyA = value;
}
}
get
{
ThingType propertyA;
lock(_Lock)
{
propertyA = _PropertyA;
}
}
}
private ThingType _PropertyA = new ThingType();
private object _Lock = new object();
}Software Zen:
delete this;
Fold With Us![^] -
supercat9 wrote:
if a lock must be held between calls to an objects methods or properties, it must be publicly available.
That isn't quite true. An instance member marked
privatecan be the lock value, manipulated solely by members of the class, like this:class ThreadedWhatsit
{
public ThreadedWhatsit
{
lock(_Lock)
{
// do stuff
}
}
public ThingType PropertyA
{
set
{
lock(_Lock)
{
_PropertyA = value;
}
}
get
{
ThingType propertyA;
lock(_Lock)
{
propertyA = _PropertyA;
}
}
}
private ThingType _PropertyA = new ThingType();
private object _Lock = new object();
}Software Zen:
delete this;
Fold With Us![^]Gary R. Wheeler wrote:
An instance member marked private can be the lock value, manipulated solely by members of the class, like this
In your example, the lock is released between calls to the object's functions. If you used the Monitor.* functions, you could have have your code hold a lock when a function returns, but doing so would be even more dangerous than publicly exposing the lock object. The main danger of publicly exposing a lock object is that there's no way the code for the class to protect against deadlock. Having a function which returns while a lock is held poses a much bigger danger, which is that the lock might be accidentally abandoned while it is held.
-
I have no idea what he locks in this C# code.
// path is a local variable.
lock (path)
{
if (File.Exists(path))
{
File.Delete(path);
}
}Maybe the variable "path" is only used for locking against multiple threads running this at the same time. If they assign a value to the variable, that would be kinda weird. I use the "lock" keyword for multithreading, but I usually name the variable something like "padlock" and don't use it for anything else.
-
Gary R. Wheeler wrote:
An instance member marked private can be the lock value, manipulated solely by members of the class, like this
In your example, the lock is released between calls to the object's functions. If you used the Monitor.* functions, you could have have your code hold a lock when a function returns, but doing so would be even more dangerous than publicly exposing the lock object. The main danger of publicly exposing a lock object is that there's no way the code for the class to protect against deadlock. Having a function which returns while a lock is held poses a much bigger danger, which is that the lock might be accidentally abandoned while it is held.
My point is that the class manages the object the lock is protecting. All access to the object are through the class, so therefore the object is protected. Publicly exposing a lock, or holding the lock across method calls, is insanely poor design.
Software Zen:
delete this;
Fold With Us![^] -
My point is that the class manages the object the lock is protecting. All access to the object are through the class, so therefore the object is protected. Publicly exposing a lock, or holding the lock across method calls, is insanely poor design.
Software Zen:
delete this;
Fold With Us![^]Publicly exposing a lock, or holding the lock across method calls, is insanely poor design.
Agreed, but if one wishes to allow people to nicely enumerate collections I'm not sure there's always a good way around it. Personally, I wish that Microsoft's contract had allowed collections to be changed during an enumeration subject to some restrictions (basically, things which exist throughout an enumeration must be returned once; things that exist for part of an enumeration may be returned at most once) but that's not how Microsoft specified it. I am well aware that locking a collection during enumeration is a dangerous recipe for deadlock, but there isn't always a practical alternative.