Thursday, January 22, 2009

Access Violations, Interop and VS2008 SP1

After installing SP1 for VS2008 the other day, one of our programs stopped working. In fact, it came up with an AccessViolationException:

System.AccessViolationException: Attempted to read or write protected
memory. This is often an indication that other memory is corrupt.
at VariantClear(tagVARIANT* pvarg)
at
System.Runtime.InteropServices.CustomMarshalers.EnumeratorViewOfEnumVariant.MoveNext()
at ...etc...

This was a surprise, because it ran fine before. Or, at least, that is what we thought.

An examination of the code revealed the exception was being thrown in a foreach() statement. Odd, but the foreach() statement was looping on a COM Interop collection. I've done a little Outlook Add-In development and one thing I've learnt about COM Interop is that it doesn't handle memory the way you would think .NET handles it.

Basically, there are two thing you need to do:
1) Do NOT use foreach() - these are bad, use for(;;)
2) Be very careful about releasing memory for references to COM objects behind Interop interfaces

There is a great article regarding this here:

OOM.NET: Part 2 - Outlook Item Leaks

If you do any Interop work, you really should read and understand it.

Back to the code that failed:

     private static bool ColIsForeignKey(Table t, Column col)
{
foreach (Key key in t.Keys)
{
if (key.Type != SQLDMO_KEY_TYPE.SQLDMOKey_Foreign) continue;
foreach (string keyCol in key.KeyColumns)
{
if (keyCol.ToLower() == col.Name.ToLower())
{
return true;
}
}
}
return false;
}

Now here is the code modified:
     private static bool ColIsForeignKey(Table t, Column col)
{
SQLDMO.Keys keys = t.Keys;
try
{
for (int i = 1; i <= keys.Count; i++)
{
Key key = keys.Item(i);
try
{
if (key.Type != SQLDMO_KEY_TYPE.SQLDMOKey_Foreign) continue;

Names columns = key.KeyColumns;
try
{
for (int j = 1; j <= columns.Count; j++)
{
string keyCol = columns.Item(j);
if (keyCol.ToLower() == col.Name.ToLower())
{
return true;
}
}
}
finally
{
Marshal.ReleaseComObject(columns);
columns = null;
}
}
finally
{
Marshal.ReleaseComObject(key);
key = null;
}
}
}
finally
{
Marshal.ReleaseComObject(keys);
keys = null;
}
return false;
}

It looks a lot uglier, but you can be sure you're not keeping any nasty
references around the place.