Is String.IsNullOrEmpty Hazardous to your Code's health in .Net 2.0/3.0?

Posted by OmegaMan at August 24, 2007

Category: .Net, Bug Reports

While working in the forums a user reported that the use of String.IsNullOrEmpty could cause your code to generate an exception due to its use in certain circumstances and should not be used! 
 
Is that true? Well yes but no…and that no is a real qualified No. Let me explain why you should not even be concerned about this and continue using IsNullOrEmpty without any concerns. The issue was reported in this connect issue entitled Null Exemption caused by JIt optimisation around String.IsNullOrEmpty. Don’t read it yet, let me save you some time reading it.
 
The user stated that he had discovered that if you placed String.IsNullOrEmpty in a tight loop, and I mean tight, then compiled it for release with optimizations it would generate an exception; a System.NullReferenceException. Here is an example. Note I simplified the user’s example of the code which will do it:
 
string Fail = null;
 
for (int index = 0; index < 10; index++)
    if (string.IsNullOrEmpty(Fail))
    { 
        // Yes leave this empty
    } 

Yes it fails, and Microsoft, even though it was reported in 06, said they would fix it until what is now .Net 3.5.

People are up in arms on this issue….

But wait. There is a simple fix and the fix is not a fix because the code above is atypical in how someone actually programs and no real fix is required!

Here is how one can fix it:

string Fail = null;
 
for (int index = 0; index < 10; index++)
    if (string.IsNullOrEmpty(Fail))
        Console.Write(index);

In other words, don’t program empty constructs and you won’t have any problems! By simply doing work within the loop and not having an empty construct fixes the non problem.

People are up in arms that this should be fixed immediately! The nay-saying neighbobs of negativism are saying that a patch should be immediately done! Why hasn’t Microsoft fixed this. Some are suggesting that code out in the field at client sites are prone to failure and clients will be lost.

Oh, please…to reference a Forest Gump quote, “Programming is as Programming does”. If one is creating empty constructs and leaving it in code, that’s analogous to the person walking along the train tracks listening to music…who’s fault is it that they did not hear the train….hmmmm?

Share

12 Comments

  1. Todd Lehr says

    Doesn’t seem like a big deal though i can get it throw an exception on the code below. Couldn’t get it throw exception iterating through a string array or a foreach loop where it would be useful.

    string s = null;
    int nonNullWordCount = 0;

    for(int i=0; i

    Reply
  2. Todd Lehr says

    for(int i=0; i > 100;i++)
    {

    if (!string.IsNullOrEmpty(s))
    {
    nonNullWordCount++;
    }
    }

    It won’t throw an exception if you replace nonNullWordCount++ with a Console.Writeline even though it would never be called. Something weird happens with Console.Write.

    Reply
  3. omegaman says

    That is an interesting post. I will definitely have to look into it…unfortunately I am heading out on vacation. When I return I will give it a test run! Thanks Todd!

    Reply
  4. Niki says

    > Hi Niki, the blog interface is not kind to commmenters. Please repost!

    Ok, last try: Consider this code:
    static void test(string x)
    {
    int characterCount = 0;
    for (int j = 0; j != 10; j++)
    {
    if (String.IsNullOrEmpty(x))
    continue;
    characterCount += x.Length;
    }
    Console.WriteLine(characterCount.ToString());
    }
    It does something in the “then”-clause, and in the rest of the loop; I’d say this kind of code is fairly typical if you’re doing string processing – but it crashes in a release build!

    My guess is that Console.Write (in your example) can’t be inlined, which blocks the buggy optimization from happening. But the bug is still there, and I’m still “up in arms” on it. And I can’t really believe they actually *aren’t* going to fix it.
    [Insert snappy remark about MS quality standards here]

    Reply
  5. omegaman says

    I can’t agree that your code snippet is typical; only that it proves your point.

    An optimizer by its vary nature removes redundant code. Every example I have seen in researching this, the code is an illogical construct. Meaning that if I saw it in code I would rewrite it!

    But you hit the nail on the head when you said ConsoleWriteline does something; yes it and a vast majority other code has the same reaction by the optimizer. It is not reduced by the the optimizer.

    I appreciate your comments, but I have yet to see a real world example where this fails. Because real world code is simply not reduce by optimizations.

    Reply
  6. Niki says

    > I appreciate your comments, but I have yet to see a real world example

    Somebody found this problem, and I’m willing to bet that they found it after hours, maybe days of debugging production code. I don’t envy them either, because debugging a problem that only happens in a release build is tough enough, and I’d never expect a method named “IsNullOrEmpty” to throw a NullReferenceException. So they simplified the problem for a bug report by stripping away anything that’s not neccessary to reproduce the bug. And now you think this is a non-issue because it’s too simple for a real-world app? Would you rather have everyone who finds a bug in the CLR upload their full source tree, with a couple of pages on how to reproduce the problem??

    > I appreciate your comments, but I have yet to see a real world
    > example where this fails. Because real world code is simply not
    > reduce by optimizations.

    Why do you think MS built this optimization in the first place, if they didn’t expect it to bring some performance benefit? Inling and variable-enregistration are probably the most effective – and thus most important – optimizations there are, I don’t like the idea that the optimizer might choose to “optimize” some of my x=null-conditions away…

    But I think the most important point is: neither you nor I have the source code to the JIT optimizer, so neither of us can really be sure what circumstances are neccessary for this bug. You’ve made a guess in your blog, and I think I’ve shown your guess was wrong, so as far as I can see, we still don’t know when this bug can happen, and all we can do is keep guessing; You seem to be the optimistic type, guessing there won’t be a problem until you seen one, but – no offense – my experience is that a pessistic attitude towards unknowns is usually better if you want to create reliable code…

    If someone could give me a detailed, verifiable analysis that explains precisely why and under what circumstances this bug happens then I might be convinced that this is indeed a non-issue. Until that point, I’ll be a “nay-saying neighbobs of negativism” and stick to my opinion that bugs should be fixed.

    Reply
  7. omegaman says

    Well said Niki! I can’t argue with your logic on any of the points and actually quite agree. It is interesting to note that Microsoft, to my knowledge, did not release a hot fix for it though….does that imply no one (maybe big enough) has yelled for the fix? I leave you with that. Thanks again for your thoughts.

    Reply
  8. String.IsNullOrEmpty Shootout : DevTopics says

    […] The time difference is minimal enough that you can safely choose either option.  You may actually prefer IsNullOrEmpty because it’s more intuitive.  And the rumor about IsNullOrEmpty crashing is much ado about nothing. […]

    Reply
  9. Gregory Stein says

    Thank you guys! Especially to Niki!

    The code Niki wrote was my issue! I had to find total number of characters and this code is the exactly what I’ve written.

    Oh! I spent one day solving this! Thank you very much!!!

    P.S> Why have I switched from Java to .NET??? 🙁

    Reply
  10. Antony says

    Good debate, I guess that it suggests that refactoring functions into lowest common denominators and then wrapping exception handling round each one is necessary. Not trusting even inbuilt library methods is the safest approach.

    As an aside this website (omegacoder.com) was banned by software I was using fortinet as Pornography until I had a revision approved yesterday. I wonder if a similarly named site is a bit naughty? Hopefully there will be some kind of traffic improvement as the site is now unbanned, allowing access from a number of big companies.

    Reply
  11. Gunjan says

    Hi,

    I am using .NET 3.5.
    I am trying to read values from excel file and then using it further. I am converting the values read from excel to string and checking with string.IsNullOrEmpty function but it gives me exception at line 1 and does not go to if clause. The code is:

    name =((Range)workSheet.Cells[rowIndex,nameColIndex]).Value2.ToString();
    if (string.IsNullOrEmpty(name))
    {
    name = “”;
    }

    however if I change name to ‘null’ keyword it works fine.

    Can you please let me know if this is related to bug or help me solve it?

    Reply
    • omegaman says

      Check to see if Value2 is null. If it is not then ask it to return a string via ToString(). A null value cannot return a string.

      Reply

Leave a comment

(required)
(required) (will not be published)

This site uses Akismet to reduce spam. Learn how your comment data is processed.