By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
448,672 Members | 1,628 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 448,672 IT Pros & Developers. It's quick & easy.

Thread safety advice

P: n/a
My application uses a singleton static class for writing entries to a log
file. The location and name of the log-file is read from web.config each time
an entry is written, but has the current date inserted into its name. For
example, the string:

c:\inetpub\wwwroot\myapp\log.txt

will be changed to:

c:\inetpub\wwwroot\myapp\log_20060216.txt

However, as we have many concurrent users on the site at once, each
generating hundreds of log file entries a minute, we're finding that multiple
threads are calling the same code at the same time and we're getting filename
with multiple date stamps. Ie:

c:\inetpub\wwwroot\myapp\log_20060216_20060216.txt

I'm sure this is to do with the following code not being thread-safe despite
the "lock" critical section statement:

string sLog_File_Path = ConfigurationManager.AppSettings["logfilepath"];

lock (FoLockObject) {
sLog_File_Path = Path.Combine(Path.GetDirectoryName(sLog_File_Path) ,
Path.GetFileNameWithoutExtension(sLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(sLog_File_Path));
}

Can anyone offer advice on the correct way to protect code that is
vulnerable to threading issues. I'm aware of synchronization objects such as
Mutex's but am unsure how to use them with static classes. A stright forward
example in C# without being too clever would be very handy

Thanks

Ben

Feb 16 '06 #1
Share this Question
Share on Google+
4 Replies


P: n/a
Is sLog_File_Path local or member variable?
What about FoLockObject?

Unfortunately it' hard to say where is the problem you need to show us full
class.

George.
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:27**********************************@microsof t.com...
My application uses a singleton static class for writing entries to a log
file. The location and name of the log-file is read from web.config each
time
an entry is written, but has the current date inserted into its name. For
example, the string:

c:\inetpub\wwwroot\myapp\log.txt

will be changed to:

c:\inetpub\wwwroot\myapp\log_20060216.txt

However, as we have many concurrent users on the site at once, each
generating hundreds of log file entries a minute, we're finding that
multiple
threads are calling the same code at the same time and we're getting
filename
with multiple date stamps. Ie:

c:\inetpub\wwwroot\myapp\log_20060216_20060216.txt

I'm sure this is to do with the following code not being thread-safe
despite
the "lock" critical section statement:

string sLog_File_Path = ConfigurationManager.AppSettings["logfilepath"];

lock (FoLockObject) {
sLog_File_Path = Path.Combine(Path.GetDirectoryName(sLog_File_Path) ,
Path.GetFileNameWithoutExtension(sLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(sLog_File_Path));
}

Can anyone offer advice on the correct way to protect code that is
vulnerable to threading issues. I'm aware of synchronization objects such
as
Mutex's but am unsure how to use them with static classes. A stright
forward
example in C# without being too clever would be very handy

Thanks

Ben

Feb 16 '06 #2

P: n/a
Here's an abbreviated version of the full class.

public class CLog {
private static string FsLog_File_Path = "";
private static object FoLockObject = new object();

private static void RefreshSettings() {
FsLog_File_Path = ConfigurationSettings.AppSettings["logfilepath"];

lock (FoLockObject) {
FsLog_File_Path =
Path.Combine(Path.GetDirectoryName(FsLog_File_Path ),
Path.GetFileNameWithoutExtension(FsLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(FsLog_File_Path));
}
}

public static void WriteInfo(string sDescription) {
RefreshSettings();

StreamWriter oStream = null;

try {
if (File.Exists(FsLog_File_Path)) {
oStream = File.AppendText(FsLog_File_Path);
}
else {
oStream = File.CreateText(FsLog_File_Path);
}

oStream.AutoFlush = true;

string sSession = "";
if (HttpContext.Current != null) sSession =
HttpContext.Current.Session.SessionID + " - ";
else sSession = "";
oStream.WriteLine(string.Format("{0} - {1} - {2}{3}",
DateTime.Now.ToString("dd MMM yyy H:mm:ss"), "INFO", sSession,
sDescription));
}
catch (IOException) {

}
finally {
if (oStream != null) oStream.Close();
oStream = null;
}
}
}

"George Ter-Saakov" wrote:
Is sLog_File_Path local or member variable?
What about FoLockObject?

Unfortunately it' hard to say where is the problem you need to show us full
class.

George.
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:27**********************************@microsof t.com...
My application uses a singleton static class for writing entries to a log
file. The location and name of the log-file is read from web.config each
time
an entry is written, but has the current date inserted into its name. For
example, the string:

c:\inetpub\wwwroot\myapp\log.txt

will be changed to:

c:\inetpub\wwwroot\myapp\log_20060216.txt

However, as we have many concurrent users on the site at once, each
generating hundreds of log file entries a minute, we're finding that
multiple
threads are calling the same code at the same time and we're getting
filename
with multiple date stamps. Ie:

c:\inetpub\wwwroot\myapp\log_20060216_20060216.txt

I'm sure this is to do with the following code not being thread-safe
despite
the "lock" critical section statement:

string sLog_File_Path = ConfigurationManager.AppSettings["logfilepath"];

lock (FoLockObject) {
sLog_File_Path = Path.Combine(Path.GetDirectoryName(sLog_File_Path) ,
Path.GetFileNameWithoutExtension(sLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(sLog_File_Path));
}

Can anyone offer advice on the correct way to protect code that is
vulnerable to threading issues. I'm aware of synchronization objects such
as
Mutex's but am unsure how to use them with static classes. A stright
forward
example in C# without being too clever would be very handy

Thanks

Ben


Feb 16 '06 #3

P: n/a
You do have a problem in your code.
This line is not thread safe.

FsLog_File_Path = ConfigurationSettings.AppSettings["logfilepath"];

You are modifying the variable while some other thread could have been doing
the code that is in lock {} section.
So move that line inside (but read further)

-------------------------------------------------
I am not sure why your are doing it this way. Because I do not see any gain
in FsLog_File_Path beign global/member variable.
I would rewrite the code to avoid any synchronization

private static void RefreshSettings() {
string sPath =
ConfigurationSettings.AppSettings["logfilepath"];
sPath = Path.Combine(Path.GetDirectoryName(sPath),
Path.GetFileNameWithoutExtension(sPath) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(sPath));
}
}

As you can see all manipulations are made to local variable so you do not
need lock.

------------------------------------------------------------------

The only place where you will need to lock is where you are opening the file
and writing to it. Since that is not thread safe.
And this will fail if you try to open/write into file from multiple threads.

George
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:B6**********************************@microsof t.com...
Here's an abbreviated version of the full class.

public class CLog {
private static string FsLog_File_Path = "";
private static object FoLockObject = new object();

private static void RefreshSettings() {
FsLog_File_Path = ConfigurationSettings.AppSettings["logfilepath"];

lock (FoLockObject) {
FsLog_File_Path =
Path.Combine(Path.GetDirectoryName(FsLog_File_Path ),
Path.GetFileNameWithoutExtension(FsLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(FsLog_File_Path));
}
}

public static void WriteInfo(string sDescription) {
RefreshSettings();

StreamWriter oStream = null;

try {
if (File.Exists(FsLog_File_Path)) {
oStream = File.AppendText(FsLog_File_Path);
}
else {
oStream = File.CreateText(FsLog_File_Path);
}

oStream.AutoFlush = true;

string sSession = "";
if (HttpContext.Current != null) sSession =
HttpContext.Current.Session.SessionID + " - ";
else sSession = "";
oStream.WriteLine(string.Format("{0} - {1} - {2}{3}",
DateTime.Now.ToString("dd MMM yyy H:mm:ss"), "INFO", sSession,
sDescription));
}
catch (IOException) {

}
finally {
if (oStream != null) oStream.Close();
oStream = null;
}
}
}

"George Ter-Saakov" wrote:
Is sLog_File_Path local or member variable?
What about FoLockObject?

Unfortunately it' hard to say where is the problem you need to show us
full
class.

George.
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:27**********************************@microsof t.com...
> My application uses a singleton static class for writing entries to a
> log
> file. The location and name of the log-file is read from web.config
> each
> time
> an entry is written, but has the current date inserted into its name.
> For
> example, the string:
>
> c:\inetpub\wwwroot\myapp\log.txt
>
> will be changed to:
>
> c:\inetpub\wwwroot\myapp\log_20060216.txt
>
> However, as we have many concurrent users on the site at once, each
> generating hundreds of log file entries a minute, we're finding that
> multiple
> threads are calling the same code at the same time and we're getting
> filename
> with multiple date stamps. Ie:
>
> c:\inetpub\wwwroot\myapp\log_20060216_20060216.txt
>
> I'm sure this is to do with the following code not being thread-safe
> despite
> the "lock" critical section statement:
>
> string sLog_File_Path =
> ConfigurationManager.AppSettings["logfilepath"];
>
> lock (FoLockObject) {
> sLog_File_Path = Path.Combine(Path.GetDirectoryName(sLog_File_Path) ,
> Path.GetFileNameWithoutExtension(sLog_File_Path) + "_" +
> DateTime.Now.ToString("yyyyMMdd") +
> Path.GetExtension(sLog_File_Path));
> }
>
> Can anyone offer advice on the correct way to protect code that is
> vulnerable to threading issues. I'm aware of synchronization objects
> such
> as
> Mutex's but am unsure how to use them with static classes. A stright
> forward
> example in C# without being too clever would be very handy
>
> Thanks
>
> Ben
>


Feb 16 '06 #4

P: n/a
Hi George,

I see the error of my ways and have changed it as you suggested. This was
basically a quick hack to include the date in the filename, as this wasn't
the origianl intention.

Thanks for the advice

Ben
"George Ter-Saakov" <gt****@cardone.com> wrote in message
news:OO**************@TK2MSFTNGP10.phx.gbl...
You do have a problem in your code.
This line is not thread safe.

FsLog_File_Path = ConfigurationSettings.AppSettings["logfilepath"];

You are modifying the variable while some other thread could have been
doing the code that is in lock {} section.
So move that line inside (but read further)

-------------------------------------------------
I am not sure why your are doing it this way. Because I do not see any
gain in FsLog_File_Path beign global/member variable.
I would rewrite the code to avoid any synchronization

private static void RefreshSettings() {
string sPath =
ConfigurationSettings.AppSettings["logfilepath"];
sPath = Path.Combine(Path.GetDirectoryName(sPath),
Path.GetFileNameWithoutExtension(sPath) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(sPath));
}
}

As you can see all manipulations are made to local variable so you do not
need lock.

------------------------------------------------------------------

The only place where you will need to lock is where you are opening the
file and writing to it. Since that is not thread safe.
And this will fail if you try to open/write into file from multiple
threads.

George
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:B6**********************************@microsof t.com...
Here's an abbreviated version of the full class.

public class CLog {
private static string FsLog_File_Path = "";
private static object FoLockObject = new object();

private static void RefreshSettings() {
FsLog_File_Path =
ConfigurationSettings.AppSettings["logfilepath"];

lock (FoLockObject) {
FsLog_File_Path =
Path.Combine(Path.GetDirectoryName(FsLog_File_Path ),
Path.GetFileNameWithoutExtension(FsLog_File_Path) + "_" +
DateTime.Now.ToString("yyyyMMdd") +
Path.GetExtension(FsLog_File_Path));
}
}

public static void WriteInfo(string sDescription) {
RefreshSettings();

StreamWriter oStream = null;

try {
if (File.Exists(FsLog_File_Path)) {
oStream = File.AppendText(FsLog_File_Path);
}
else {
oStream = File.CreateText(FsLog_File_Path);
}

oStream.AutoFlush = true;

string sSession = "";
if (HttpContext.Current != null) sSession =
HttpContext.Current.Session.SessionID + " - ";
else sSession = "";
oStream.WriteLine(string.Format("{0} - {1} - {2}{3}",
DateTime.Now.ToString("dd MMM yyy H:mm:ss"), "INFO", sSession,
sDescription));
}
catch (IOException) {

}
finally {
if (oStream != null) oStream.Close();
oStream = null;
}
}
}

"George Ter-Saakov" wrote:
Is sLog_File_Path local or member variable?
What about FoLockObject?

Unfortunately it' hard to say where is the problem you need to show us
full
class.

George.
"Ben Fidge" <Be******@discussions.microsoft.com> wrote in message
news:27**********************************@microsof t.com...
> My application uses a singleton static class for writing entries to a
> log
> file. The location and name of the log-file is read from web.config
> each
> time
> an entry is written, but has the current date inserted into its name.
> For
> example, the string:
>
> c:\inetpub\wwwroot\myapp\log.txt
>
> will be changed to:
>
> c:\inetpub\wwwroot\myapp\log_20060216.txt
>
> However, as we have many concurrent users on the site at once, each
> generating hundreds of log file entries a minute, we're finding that
> multiple
> threads are calling the same code at the same time and we're getting
> filename
> with multiple date stamps. Ie:
>
> c:\inetpub\wwwroot\myapp\log_20060216_20060216.txt
>
> I'm sure this is to do with the following code not being thread-safe
> despite
> the "lock" critical section statement:
>
> string sLog_File_Path =
> ConfigurationManager.AppSettings["logfilepath"];
>
> lock (FoLockObject) {
> sLog_File_Path = Path.Combine(Path.GetDirectoryName(sLog_File_Path) ,
> Path.GetFileNameWithoutExtension(sLog_File_Path) + "_" +
> DateTime.Now.ToString("yyyyMMdd") +
> Path.GetExtension(sLog_File_Path));
> }
>
> Can anyone offer advice on the correct way to protect code that is
> vulnerable to threading issues. I'm aware of synchronization objects
> such
> as
> Mutex's but am unsure how to use them with static classes. A stright
> forward
> example in C# without being too clever would be very handy
>
> Thanks
>
> Ben
>


Feb 16 '06 #5

This discussion thread is closed

Replies have been disabled for this discussion.