One thing I need to do to get Grid FX working in Visual Studio 2008 is to render all style attributes inline, as opposed to the embedded css classes that it was currently doing. Anyway, I spent the better part of my day changing the system (for the better in my opinion, which makes me happy). No real problems until I got a strange IndexOutOfRangeException when I called HtmlTextWriter.AddStyleAttribute more than 20 times. The HtmlTextWriter does just what its name implies - it writes the HTML for the control. The style attribute and value were perfectly valid. I couldn't find anything wrong on my side.
A while ago I used a Reflector add-in to decompile most of the .NET framework DLLs. This really comes in handy when I need to know what's really going on inside the framework. So I pulled up the method in question and found the following code (omitting the irrelevant parts):
protected virtual void AddStyleAttribute(string name, string value...
{
if (this._styleList == null)
{
this._styleList = new RenderStyle[20];
}
else if (this._styleCount > this._styleList.Length)
{
RenderStyle[] styleArray1 = new RenderStyle[this._styleList.Length * 2];
Array.Copy(this._styleList, styleArray1, this._styleList.Length);
this._styleList = styleArray1;
}
this._styleList[this._styleCount] = style1;
this._styleCount++;
}
It fails on call #21 (and 41 and 61, etc) because the person who wrote it made a very simple mistake. The line "this._styleCount > this._styleList.Length" should have used >=. Wow, such a simple error. Oddly, the AddAttribute method had pretty much the exact same code without that mistake.
It's kinda scary because you would think that if you go that route of writing code to expand an array size that you would test the limits, especially if you hardcode 20 as the initial limit. Bad coding, maybe, but it's the weak testing that I blame.
I would like to point out that you probably won't ever hit this bug because you really shouldn't use a ton of inline attributes. We do it only at design time, because the css attributes from Grid's Palette and Motif can be many, mainly because they are split into individual styles (padding-top/left/right, border-top-width, etc) rather than composite attributes like padding, border, etc. So I guess we could be the first ones to ever render more than 20 style attributes. Good thing I use external style sheets at runtime and don't have to deal with the inline style mess.
Well shit, what do I do now? I suppose I could write them and point it out. But that won't do much good because the libraries can't change. So I tried some ideas and had some luck...
I used reflection to increment the "_styleCount" field when that exception occurs. This will cause the next call to AddStyleAttribute to actually run the code to double the array size, because "this._styleCount > this._styleList.Length" will evaluate to true. This is a little dangerous because the array would have an empty element, and I could get another exception down the line. Thankfully that didn't happen, because the object in the array is a struct called RenderStyle, and is never null...and the code to write out the attributes checks against a null key or value.
So I wrote some helper methods and stuck them in a utils class. Here they are. Calling these AddStyleAttribute methods instead of the writer's AddStyle attribute would be smart. Using these calls will bypass that error. I would have preferred to use C# 3.0 extension methods instead, but I can't because we compile Grid FX off framework 2.0. Oh well, this is good enough.
///
/// Calls the HtmlTextWriter's AddStyleAttribute method, bypassing an exception that will occur after the 20th call. Th
///
public static void AddStyleAttribute(HtmlTextWriter writer, HtmlTextWriterStyle key, string value)
{
try
{
writer.AddStyleAttribute(key, value);
}
catch (IndexOutOfRangeException e)
{
IncrementHtmlTextWriterStyleCountField(writer);
writer.AddStyleAttribute(key, value);
}
}
///
/// Calls the HtmlTextWriter's AddStyleAttribute method, bypassing an exception that will occur after the 20th call. Th
///
public static void AddStyleAttribute(HtmlTextWriter writer, string name, string value)
{
try
{
writer.AddStyleAttribute(name, value);
}
catch (IndexOutOfRangeException e)
{
IncrementHtmlTextWriterStyleCountField(writer);
writer.AddStyleAttribute(name, value);
}
}
///
/// Increments the HtmlTextWriter's _styleCount field. Used to bypass a bug in the framework where IndexOutOfRangeException is thrown on the 21st, 41st, 81st, etc call to AddStyleAttribute.
///
private static void IncrementHtmlTextWriterStyleCountField(HtmlTextWriter writer)
{
//as an optimization, get the FieldInfo lazily and cache it for subsequent hits, since the lookup is the major piece of work.
//a double check locking technique is used to be thread safe but only use locking if the FieldInfo hasn't been cached.
if (m_htmlTextWriter_styleCountInfo == null)
{
lock (m_htmlTextWriter_styleCountLockObj)
{
//check again for null in case two threads entered the above if block and the first one exited the critical section
if (m_htmlTextWriter_styleCountInfo == null)
{
m_htmlTextWriter_styleCountInfo = typeof(HtmlTextWriter).GetField("_styleCount", BindingFlags.Instance | BindingFlags.NonPublic);
}
}
}
int currValue = (int)m_htmlTextWriter_styleCountInfo.GetValue(writer);
m_htmlTextWriter_styleCountInfo.SetValue(writer, currValue + 1);
}
static FieldInfo m_htmlTextWriter_styleCountInfo;
static object m_htmlTextWriter_styleCountLockObj = new object();
See ya.
No comments:
Post a Comment