Is this public static method for reading files thread safe

  • Thread starter Thread starter GG
  • Start date Start date
G

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;
}
}
 
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
 
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.
 
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() )
Click to expand...
Can you please explain. I do not follow.

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

Nicholas Paldino said:
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)

William Stacey said:
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
Click to expand...
Click to expand...
 
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
 
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.
 
Back
Top