Thread safe singleton to access the cache?

I'm trying to nail down some issues with the cache in my application.
Currently, I have an object that stands between my business logic and
database logic called CacheLogic (cute, no?). Global.asax.cs creates it in
Application_Start, initializes it and places it in the cache. During
initialization, CacheLogic retrieves data from the DB logic layer and caches
it with removal callbacks set. Whenever an object in the business logic
layer needs data, it grabs the CacheLogic object from the cache (that's a
violation of the layer right there, I know), and requests the needed data
from it. CacheLogic retrieves the data from the cache and returns it. If the
data is not in the cache, it retrieves the data from the database logic
layer, caches it (with removal callbacks), and then returns the data.

I'm concerned about what happens when two clients are requesting the same
data at the same time when that data is not in the cache. Right now, as I
understand it, CacheLogic will hit the database and cache the data twice.
I'm not sure what side effects that might have, so I'm trying to figure out
ways to avoid this situation completely without killing performance (i.e.,
lock(HttpContext.Current.Cache)).

To make CacheLogic more thread safe, I can lock on a sync object in the
class when I write to the cache or use Mutex. But what use is that unless I
also lock when I read from the cache? And isn't that just as bad as locking
the cache object itself?

The other issue I have is that CacheLogic stores objects in the cache with
removal callbacks pointing to methods in the CacheLogic object. If I create
CacheLogic objects willy-nilly, I could end up with multiple objects in
memory, referenced only by the callback in some cache object. Sounds too
close to a memory leak for me. Currently, by storing a single CacheLogic
object in the cache itself, I have one single object that handles caching and
callbacks for all client connections. I'm not happy that in order to get the
CacheLogic object, other objects have to violate the layers and access the
cache directly.

Do I even need to worry about keeping a single CacheLogic object? Is
converting CacheLogic into a singleton (is there an ISingleton interface?) a
good solution to this problem? And, if I do, once I use the CacheLogic
singleton in Application_Start (assuming I code it properly), will it be
truly available to all clients who call CacheLogic.GetInstance()?you are correct need to lock the cache on read and writes. how well you
design the locking will control how mush a bottleneck it will. if the cache
is readonly, (and the cache objects are thread safe), then you only need to
synchronize access to the cache, not the objects themselves.

not sure what your removal callbacks are used for. typically caches allow
removal of any object at anytime, to allow cache pruning.

switching to a Singleton is just a design pattern (static method to access
object), it does not change your locking issues, you still need to handle
these.

-- bruce (sqlwork.com)

"William Sullivan" <WilliamSullivan@.discussions.microsoft.com> wrote in
message news:D0527EBA-8DAD-464E-92AA-558F32F4AE44@.microsoft.com...
> I'm trying to nail down some issues with the cache in my application.
> Currently, I have an object that stands between my business logic and
> database logic called CacheLogic (cute, no?). Global.asax.cs creates it
> in
> Application_Start, initializes it and places it in the cache. During
> initialization, CacheLogic retrieves data from the DB logic layer and
> caches
> it with removal callbacks set. Whenever an object in the business logic
> layer needs data, it grabs the CacheLogic object from the cache (that's a
> violation of the layer right there, I know), and requests the needed data
> from it. CacheLogic retrieves the data from the cache and returns it. If
> the
> data is not in the cache, it retrieves the data from the database logic
> layer, caches it (with removal callbacks), and then returns the data.
> I'm concerned about what happens when two clients are requesting the same
> data at the same time when that data is not in the cache. Right now, as I
> understand it, CacheLogic will hit the database and cache the data twice.
> I'm not sure what side effects that might have, so I'm trying to figure
> out
> ways to avoid this situation completely without killing performance (i.e.,
> lock(HttpContext.Current.Cache)).
> To make CacheLogic more thread safe, I can lock on a sync object in the
> class when I write to the cache or use Mutex. But what use is that unless
> I
> also lock when I read from the cache? And isn't that just as bad as
> locking
> the cache object itself?
> The other issue I have is that CacheLogic stores objects in the cache with
> removal callbacks pointing to methods in the CacheLogic object. If I
> create
> CacheLogic objects willy-nilly, I could end up with multiple objects in
> memory, referenced only by the callback in some cache object. Sounds too
> close to a memory leak for me. Currently, by storing a single CacheLogic
> object in the cache itself, I have one single object that handles caching
> and
> callbacks for all client connections. I'm not happy that in order to get
> the
> CacheLogic object, other objects have to violate the layers and access the
> cache directly.
> Do I even need to worry about keeping a single CacheLogic object? Is
> converting CacheLogic into a singleton (is there an ISingleton interface?)
> a
> good solution to this problem? And, if I do, once I use the CacheLogic
> singleton in Application_Start (assuming I code it properly), will it be
> truly available to all clients who call CacheLogic.GetInstance()?

Thread safe singleton to access the cache?

I'm trying to nail down some issues with the cache in my application.
Currently, I have an object that stands between my business logic and
database logic called CacheLogic (cute, no?). Global.asax.cs creates it in
Application_Start, initializes it and places it in the cache. During
initialization, CacheLogic retrieves data from the DB logic layer and caches
it with removal callbacks set. Whenever an object in the business logic
layer needs data, it grabs the CacheLogic object from the cache (that's a
violation of the layer right there, I know), and requests the needed data
from it. CacheLogic retrieves the data from the cache and returns it. If th
e
data is not in the cache, it retrieves the data from the database logic
layer, caches it (with removal callbacks), and then returns the data.
I'm concerned about what happens when two clients are requesting the same
data at the same time when that data is not in the cache. Right now, as I
understand it, CacheLogic will hit the database and cache the data twice.
I'm not sure what side effects that might have, so I'm trying to figure out
ways to avoid this situation completely without killing performance (i.e.,
lock(HttpContext.Current.Cache)).
To make CacheLogic more thread safe, I can lock on a sync object in the
class when I write to the cache or use Mutex. But what use is that unless I
also lock when I read from the cache? And isn't that just as bad as lockin
g
the cache object itself?
The other issue I have is that CacheLogic stores objects in the cache with
removal callbacks pointing to methods in the CacheLogic object. If I create
CacheLogic objects willy-nilly, I could end up with multiple objects in
memory, referenced only by the callback in some cache object. Sounds too
close to a memory leak for me. Currently, by storing a single CacheLogic
object in the cache itself, I have one single object that handles caching an
d
callbacks for all client connections. I'm not happy that in order to get th
e
CacheLogic object, other objects have to violate the layers and access the
cache directly.
Do I even need to worry about keeping a single CacheLogic object? Is
converting CacheLogic into a singleton (is there an ISingleton interface?) a
good solution to this problem? And, if I do, once I use the CacheLogic
singleton in Application_Start (assuming I code it properly), will it be
truly available to all clients who call CacheLogic.GetInstance()?you are correct need to lock the cache on read and writes. how well you
design the locking will control how mush a bottleneck it will. if the cache
is readonly, (and the cache objects are thread safe), then you only need to
synchronize access to the cache, not the objects themselves.
not sure what your removal callbacks are used for. typically caches allow
removal of any object at anytime, to allow cache pruning.
switching to a Singleton is just a design pattern (static method to access
object), it does not change your locking issues, you still need to handle
these.
-- bruce (sqlwork.com)
"William Sullivan" <WilliamSullivan@.discussions.microsoft.com> wrote in
message news:D0527EBA-8DAD-464E-92AA-558F32F4AE44@.microsoft.com...
> I'm trying to nail down some issues with the cache in my application.
> Currently, I have an object that stands between my business logic and
> database logic called CacheLogic (cute, no?). Global.asax.cs creates it
> in
> Application_Start, initializes it and places it in the cache. During
> initialization, CacheLogic retrieves data from the DB logic layer and
> caches
> it with removal callbacks set. Whenever an object in the business logic
> layer needs data, it grabs the CacheLogic object from the cache (that's a
> violation of the layer right there, I know), and requests the needed data
> from it. CacheLogic retrieves the data from the cache and returns it. If
> the
> data is not in the cache, it retrieves the data from the database logic
> layer, caches it (with removal callbacks), and then returns the data.
> I'm concerned about what happens when two clients are requesting the same
> data at the same time when that data is not in the cache. Right now, as I
> understand it, CacheLogic will hit the database and cache the data twice.
> I'm not sure what side effects that might have, so I'm trying to figure
> out
> ways to avoid this situation completely without killing performance (i.e.,
> lock(HttpContext.Current.Cache)).
> To make CacheLogic more thread safe, I can lock on a sync object in the
> class when I write to the cache or use Mutex. But what use is that unless
> I
> also lock when I read from the cache? And isn't that just as bad as
> locking
> the cache object itself?
> The other issue I have is that CacheLogic stores objects in the cache with
> removal callbacks pointing to methods in the CacheLogic object. If I
> create
> CacheLogic objects willy-nilly, I could end up with multiple objects in
> memory, referenced only by the callback in some cache object. Sounds too
> close to a memory leak for me. Currently, by storing a single CacheLogic
> object in the cache itself, I have one single object that handles caching
> and
> callbacks for all client connections. I'm not happy that in order to get
> the
> CacheLogic object, other objects have to violate the layers and access the
> cache directly.
> Do I even need to worry about keeping a single CacheLogic object? Is
> converting CacheLogic into a singleton (is there an ISingleton interface?)
> a
> good solution to this problem? And, if I do, once I use the CacheLogic
> singleton in Application_Start (assuming I code it properly), will it be
> truly available to all clients who call CacheLogic.GetInstance()?
>

Thread safety advice

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

BenIs 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" <BenFidge@.discussions.microsoft.com> wrote in message
news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.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
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" <BenFidge@.discussions.microsoft.com> wrote in message
> news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.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
>
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" <BenFidge@.discussions.microsoft.com> wrote in message
news:B6D5463F-61F6-41BB-B39D-7679E2587C0A@.microsoft.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" <BenFidge@.discussions.microsoft.com> wrote in message
>> news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.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
>>>
>>
>
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-nsp@.cardone.com> wrote in message
news:OOv8TCwMGHA.3732@.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" <BenFidge@.discussions.microsoft.com> wrote in message
> news:B6D5463F-61F6-41BB-B39D-7679E2587C0A@.microsoft.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" <BenFidge@.discussions.microsoft.com> wrote in message
>>> news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.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
>>>>>
>>>
>>>

Thread safety advice

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 tim
e
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 multipl
e
threads are calling the same code at the same time and we're getting filenam
e
with multiple date stamps. Ie:
c:\inetpub\wwwroot\myapp\log_20060216_20
060216.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_Pa
th) + "_" +
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
BenIs 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" <BenFidge@.discussions.microsoft.com> wrote in message
news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.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_20
060216.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_Pa
th) + "_" +
> 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
>
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_P
ath) + "_" +
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 fu
ll
> class.
> George.
>
> "Ben Fidge" <BenFidge@.discussions.microsoft.com> wrote in message
> news:27A2FF7B-4565-40A5-8D95-CE197DE94166@.microsoft.com...
>
>
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" <BenFidge@.discussions.microsoft.com> wrote in message
news:B6D5463F-61F6-41BB-B39D-7679E2587C0A@.microsoft.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_P
ath) + "_" +
> 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:
>
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-nsp@.cardone.com> wrote in message
news:OOv8TCwMGHA.3732@.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" <BenFidge@.discussions.microsoft.com> wrote in message
> news:B6D5463F-61F6-41BB-B39D-7679E2587C0A@.microsoft.com...
>