Is this public static method for reading files thread safe

[GG]

Is this public static method thread safe.

//Receives a file name as a parameter
//and returns the contents of that file as a string
public static string FileToStr(string fileName)
{
FileStream fStream=null;
lock(fStream) //just in case we use it for multithreading to be thread
safe
{
StreamReader rRead = null;
string fileData=null;
try
{
fStream= new FileStream(fileName,
FileMode.Open, FileAccess.Read, FileShare.Read);
rRead = new StreamReader(fStream,System.Text.Encoding.ASCII);
fileData=rRead.ReadToEnd();
}
catch(Exception eFile)
{
throw eFile;
}
finally
{
rRead.Close();
fStream.Close();
}
return fileData;
}
}

 

[William Stacey]

Not really. Your locking a local instance of a filestream object that will
be a different object on each call. Lock a static reference type field
instead. (ie. public static lockobj = new object() )
--wjs mvp

 

[Nicholas Paldino [.NET/C# MVP]]

Actually, the method is thread safe, as it is using only local
variables.

However, there is an error in the code. If you try and call the lock
statement on a reference that is null, you should get a
NullReferenceException.

Because the method is using only local variables, the lock can be
removed, and it should work just fine.

Hope this helps.

 

[GG]

Not really. Your locking a local instance of a filestream >object that
will be a different object on each call.
So I do not need any lock if the same app through multiple threads is
calling the same method?

Lock a static reference type field
instead. (ie. public static lockobj = new object() )

Can you please explain. I do not follow.

Thanks

 

[William Stacey]

I assume his intention for the lock, was have exclusive lock to the file
(which he could do other ways). If that was not the case, he would not need
to use a lock at all and use local vars as you said, which would be thread
safe.

Actually, the method is thread safe, as it is using only local
variables.

However, there is an error in the code. If you try and call the lock
statement on a reference that is null, you should get a
NullReferenceException.

Because the method is using only local variables, the lock can be
removed, and it should work just fine.

Hope this helps.

--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Not really. Your locking a local instance of a filestream object that will
be a different object on each call. Lock a static reference type field
instead. (ie. public static lockobj = new object() )
--wjs mvp

 

[William Stacey]

Not sure what your trying to lock here. Is your intention to allow only one
thread to access the method at any one time? If so, as the method is
static, you can't lock on an instance field, like "this" or other. You can
lock on the class itself (i.e. lock(MyClass) IIRC) or create a static field
and lock on it.
Doing this from memory here:
public class MyClass
{
public static readonly object mySyncLock = new object(); //Set once.
private int someInt;
//...
public MyClass()
{
}
public static bool MyMethod(int myInt)
{
//Do stuff not needing the lock if needed.
lock(mySyncLock)
{
//Do stuff that requires exclusive access to some resource.
}// exits lock
//Do other stuff not needing the lock if needed.
}
}
HTH
--wjs

 

[GG]

Thanks everybody for clearing it up for me.

My intention is not to have exclusive lock to the file
but instead to call the same static method from the same app with
multiple threads and be thread safe.

As you recommented I will remove the lock and will do what I need.

Thanks again.