[Mono-devel-list] Patch for NullReferencException in System.Web.Caching
Eyal Alaluf
eyala at mainsoft.com
Tue Apr 26 13:23:40 EDT 2005
Hi.
We have seen recently in several applications that use System.Web.Caching that
they have an occasional NullReferenceException. This was also reported a few
times to the mono-list.
After investigation and creation of a test we found the problem to be related
to use of Sliding-Window expiration in the cache (as opposed to use of
abslute time expiration).
Attached is a stress test and a patch for System.Web.Caching that makes the
test pass.
During the test I found out that the ReaderWriterLock was unstable so I am
attaching a patch for that. I am less sure about the completeness of this
patch (it takes more time and care to fix these type of issues) but using
it I was able to well test the System.Web.Caching fixes. If you will get
an ApplicationException from the ReaderWriter lock complaining that the thread
does not have the lock (read or write) then apply this patch and retry.
The test application gets three arguments:
<number of threads>
<expiration time in millis>
[<optional argument to specify sliding-window expiration>]
For example: 'mono CacheStress 100 10 1'
will run the test using 100 threads and a 10 mills sliding expiration window.
and 'mono CacheStress 100 10'
will run the test using 100 threads and a 10 mills abosolute expiration window.
Eyal.
-------------- next part --------------
Index: Test/System.Web.Caching/CacheStress.cs
===================================================================
--- Test/System.Web.Caching/CacheStress.cs (revision 0)
+++ Test/System.Web.Caching/CacheStress.cs (revision 0)
@@ -0,0 +1,85 @@
+using System;
+using System.Web.Caching;
+using System.Threading;
+
+/// <summary>
+/// Summary description for Class1.
+/// </summary>
+public class CacheStress
+{
+ static int threads = 0;
+ static int KeyStart = 0;
+ static long SlidingWindow = 0;
+ static bool UseAbsoluteTime = false;
+ static int Modulo = 71;
+ static Cache c;
+ static SafeSum Sum = new SafeSum();
+
+ /// <summary>
+ /// The main entry point for the application.
+ /// </summary>
+ static void Main(string[] args)
+ {
+ if (args.Length < 2) {
+ Console.WriteLine("Usage: CacheStress <#threads> <#millis> [UseAbsoluteTime]");
+ return;
+ }
+ c = new Cache();
+ threads = System.Int32.Parse(args[0]);
+ SlidingWindow = System.Int64.Parse(args[1]);
+ UseAbsoluteTime = (args.Length > 2);
+ for (int i = 0; i < threads; i++)
+ {
+ Thread th = new Thread(new ThreadStart(RunCycle));
+ th.Start();
+ }
+ int secs = 10;
+ for (int j = secs; ;j += secs)
+ {
+ Thread.Sleep(1000 * secs);
+ Console.WriteLine("Executed {0} transactions in {1} seconds", Sum.Value, j);
+ }
+ }
+
+ static void RunCycle()
+ {
+ int n = Interlocked.Increment(ref KeyStart);
+ for (int i = 1; ; i++) {
+ try
+ {
+ string key = "stam" + n;
+ object o2 = c.Get(key);
+ if (o2 == null)
+ {
+ if (UseAbsoluteTime)
+ c.Insert(key, 1, null, DateTime.Now.AddTicks(SlidingWindow), Cache.NoSlidingExpiration);
+ else
+ c.Insert(key, 1, null, Cache.NoAbsoluteExpiration, new TimeSpan(SlidingWindow));
+ }
+ n = (n * 2 + i) % Modulo;
+ }
+ catch (Exception e)
+ {
+ Console.WriteLine("Caught exception " + e.GetType().ToString() + ": " + e.Message + e.StackTrace);
+ }
+ if (i == 100)
+ {
+ Sum.Add(i);
+ i = 0;
+ }
+ }
+ }
+
+ class SafeSum
+ {
+ public SafeSum()
+ {
+ _value = 0;
+ }
+
+ public int Value { get { lock(this) { return _value; } } }
+ public void Add(int i) { lock(this) { _value += i; } }
+
+ private int _value;
+ }
+}
Index: Test/System.Web.Caching/readme.txt
===================================================================
--- Test/System.Web.Caching/readme.txt (revision 0)
+++ Test/System.Web.Caching/readme.txt (revision 0)
@@ -0,0 +1,10 @@
+The CacheStress.cs test is a standalone test that should be compiled and run
+as a console application.
+In normal mode the test prints every 10 seconds the number of transactions it
+committed.
+In case of an exception the test prints the exception and continues. In case
+of a deadlock the transaction count will remain constant.
+Note that the test does not run in .Net on Windows since the System.Web.Caching
+of .Net cannot be used from a console application. Mono's implementation does
+not currently have this dependency. When (and if) it does this test should
+be made into a Web application.
Index: System.Web.Caching/ExpiresBuckets.cs
===================================================================
--- System.Web.Caching/ExpiresBuckets.cs (revision 43598)
+++ System.Web.Caching/ExpiresBuckets.cs (working copy)
@@ -37,7 +37,6 @@
/// </summary>
internal struct ExpiresEntry {
internal CacheEntry Entry;
- internal long TicksExpires;
internal int _intNext;
}
@@ -88,7 +87,6 @@
int intPos = 0;
do {
_arrEntries[intPos]._intNext = intPos + 1;
- _arrEntries[intPos].TicksExpires = Cache.NoAbsoluteExpiration.Ticks;
intPos++;
} while (intPos < _intSize);
@@ -116,7 +114,6 @@
// Initialize positions for the rest of new elements.
for (int i = oldsize; i < _intSize; i++) {
newlist[i]._intNext = i + 1;
- newlist[i].TicksExpires = Cache.NoAbsoluteExpiration.Ticks;
}
// Last item signals the expansion of the list.
@@ -147,7 +144,6 @@
}
}
- _arrEntries[_intNext].TicksExpires = objEntry.Expires;
_arrEntries[_intNext].Entry = objEntry;
objEntry.ExpiresBucket = _byteID;
@@ -172,13 +168,10 @@
/// </summary>
/// <param name="objEntry">Cache entry to be removed.</param>
internal void Remove(CacheEntry objEntry) {
- // Check if this is our bucket
- if (objEntry.ExpiresBucket != _byteID) return;
- if (objEntry.ExpiresIndex == CacheEntry.NoIndexInBucket) return;
-
_lock.AcquireWriterLock(-1);
try {
- if (_arrEntries.Length < objEntry.ExpiresIndex) return;
+ if (objEntry.ExpiresIndex == CacheEntry.NoIndexInBucket) return;
+
_intCount--;
// Push the index as a free one.
@@ -196,32 +189,6 @@
}
/// <summary>
- /// Updates a cache entry in the expires bucket, this is called during a hit of an item if the
- /// cache item has a sliding expiration. The function is responsible for updating the cache
- /// entry.
- /// </summary>
- /// <param name="objEntry">Cache entry to update.</param>
- /// <param name="ticksExpires">New expiration value for the cache entry.</param>
- internal void Update(CacheEntry objEntry, long ticksExpires) {
- // Check if this is our bucket
- if (objEntry.ExpiresBucket != _byteID) return;
- if (objEntry.ExpiresIndex == CacheEntry.NoIndexInBucket) return;
-
- _lock.AcquireWriterLock(-1);
- try {
- if (_arrEntries.Length < objEntry.ExpiresIndex) return;
-
- // Proceed to update.
- _arrEntries[objEntry.ExpiresIndex].TicksExpires = ticksExpires;
- _arrEntries[objEntry.ExpiresIndex].Entry.Expires = ticksExpires;
- }
- finally {
- //Releases both read & write locks
- _lock.ReleaseWriterLock();
- }
- }
-
- /// <summary>
/// Flushes all cache entries that has expired and removes them from the cache manager.
/// </summary>
internal void FlushExpiredItems() {
@@ -241,7 +208,7 @@
do {
objEntry = _arrEntries [intPos];
if (null != objEntry.Entry &&
- ((objEntry.TicksExpires < ticksNow) || objEntry.Entry.ExpiresBucket != _byteID))
+ ((objEntry.Entry.Expires < ticksNow) || objEntry.Entry.ExpiresBucket != _byteID))
{
if (null == removeList)
removeList = new ArrayList ();
@@ -264,6 +231,8 @@
foreach (ExpiresEntry entry in removeList) {
ExpiresEntry e = entry;
int id = entry.Entry.ExpiresIndex;
+ if (id == CacheEntry.NoIndexInBucket)
+ continue;
//push the index for reuse
_freeidx.Add (id);
@@ -657,4 +626,4 @@
#endregion
}
}
-
\ No newline at end of file
+
Index: System.Web.Caching/Cache.cs
===================================================================
--- System.Web.Caching/Cache.cs (revision 43598)
+++ System.Web.Caching/Cache.cs (working copy)
@@ -182,10 +182,9 @@
pub,
enumPriority);
- Interlocked.Increment (ref _nItems);
-
_lockEntries.AcquireWriterLock (-1);
try {
+ _nItems++;
if (_arrEntries.Contains (strKey)) {
if (overwrite)
objOldEntry = _arrEntries [strKey] as CacheEntry;
@@ -195,6 +194,16 @@
objEntry.Hit ();
_arrEntries [strKey] = objEntry;
+
+ // If we have any kind of expiration add into the CacheExpires
+ // Do this under the lock so no-one can retrieve the objEntry
+ // before it is fully initialized.
+ if (objEntry.HasSlidingExpiration || objEntry.HasAbsoluteExpiration) {
+ if (objEntry.HasSlidingExpiration)
+ objEntry.Expires = DateTime.UtcNow.Ticks + objEntry.SlidingExpiration;
+
+ _objExpires.Add (objEntry);
+ }
} finally {
_lockEntries.ReleaseLock ();
}
@@ -206,14 +215,6 @@
objOldEntry.Close (CacheItemRemovedReason.Removed);
}
- // If we have any kind of expiration add into the CacheExpires class
- if (objEntry.HasSlidingExpiration || objEntry.HasAbsoluteExpiration) {
- if (objEntry.HasSlidingExpiration)
- objEntry.Expires = DateTime.UtcNow.Ticks + objEntry.SlidingExpiration;
-
- _objExpires.Add (objEntry);
- }
-
return objEntry.Item;
}
@@ -361,6 +362,7 @@
return null;
_arrEntries.Remove (strKey);
+ _nItems--;
}
finally {
_lockEntries.ReleaseWriterLock ();
@@ -371,8 +373,6 @@
objEntry.Close (enumReason);
- Interlocked.Decrement (ref _nItems);
-
return objEntry.Item;
}
@@ -416,10 +416,7 @@
objEntry.Hit ();
if (objEntry.HasSlidingExpiration) {
- long ticksExpires = ticksNow + objEntry.SlidingExpiration;
-
- _objExpires.Update (objEntry, ticksExpires);
- objEntry.Expires = ticksExpires;
+ objEntry.Expires = ticksNow + objEntry.SlidingExpiration;
}
return objEntry;
Index: System.Web.Caching/CacheExpires.cs
===================================================================
--- System.Web.Caching/CacheExpires.cs (revision 43598)
+++ System.Web.Caching/CacheExpires.cs (working copy)
@@ -92,23 +92,6 @@
_arrBuckets [objEntry.ExpiresBucket].Remove (objEntry);
}
- internal void Update (CacheEntry objEntry, long ticksExpires) {
- // If the entry doesn't have a expires time we assume that the entry is due to expire now.
- int oldBucket = objEntry.ExpiresBucket;
- int newBucket = GetHashBucket (ticksExpires);
-
- if (oldBucket == CacheEntry.NoBucketHash)
- return;
-
- // Check if we need to move the item
- if (oldBucket != newBucket) {
- _arrBuckets [oldBucket].Remove (objEntry);
- objEntry.Expires = ticksExpires;
- _arrBuckets [newBucket].Add (objEntry);
- } else
- _arrBuckets [oldBucket].Update (objEntry, ticksExpires);
- }
-
internal void GarbageCleanup (object State) {
int bucket;
-------------- next part --------------
Index: System.Threading/ReaderWriterLock.cs
===================================================================
--- System.Threading/ReaderWriterLock.cs (revision 43144)
+++ System.Threading/ReaderWriterLock.cs (working copy)
@@ -45,7 +45,6 @@
private LockQueue writer_queue;
private Hashtable reader_locks;
private int writer_lock_owner;
- private int readyWaitingReaders = 0;
public ReaderWriterLock()
{
@@ -92,10 +91,11 @@
readers++;
try {
if (state < 0 || !writer_queue.IsEmpty) {
- if (!Monitor.Wait (this, millisecondsTimeout))
- throw new ApplicationException ("Timeout expired");
+ do {
+ if (!Monitor.Wait (this, millisecondsTimeout))
+ throw new ApplicationException ("Timeout expired");
+ } while (state < 0);
}
- readyWaitingReaders--;
}
finally {
readers--;
@@ -132,9 +132,11 @@
// wait while there are reader locks or another writer lock, or
// other threads waiting for the writer lock
- if (state != 0 || !writer_queue.IsEmpty || readers > 0) {
- if (!writer_queue.Wait (millisecondsTimeout))
- throw new ApplicationException ("Timeout expited");
+ if (state != 0 || !writer_queue.IsEmpty) {
+ do {
+ if (!writer_queue.Wait (millisecondsTimeout))
+ throw new ApplicationException ("Timeout expited");
+ } while (state != 0);
}
state = -initialLockCount;
@@ -163,7 +165,6 @@
state = lockCookie.ReaderLocks;
reader_locks [Thread.CurrentThreadId] = state;
if (readers > 0) {
- readyWaitingReaders = readers;
Monitor.PulseAll (this);
}
@@ -200,6 +201,7 @@
return;
}
}
+ Console.WriteLine("State is {0}, readers {1}", state, readers);
throw new ApplicationException ("The thread does not have any reader or writer locks.");
}
}
@@ -214,7 +216,7 @@
reader_locks [Thread.CurrentThreadId] = new_count;
state -= releaseCount;
- if (state == 0 && (readers == 0 || readyWaitingReaders <= 0) && !writer_queue.IsEmpty)
+ if (state == 0 && !writer_queue.IsEmpty)
writer_queue.Pulse ();
}
@@ -233,7 +235,6 @@
state += releaseCount;
if (state == 0) {
if (readers > 0) {
- readyWaitingReaders = readers;
Monitor.PulseAll (this);
}
else if (!writer_queue.IsEmpty)
More information about the Mono-devel-list
mailing list