Quote


A good programmer is a lazy programmer, because he writes minimum code.
--Anonymous


Showing posts with label C#. Show all posts
Showing posts with label C#. Show all posts

Monday, January 18, 2010

Exception Handling Blunders

When I do code review, one area I give at most care is the Exception Handling. There are two reasons for this.
  1. Exception handling is the least tested area in the code.
  2. Problems here can give you a few sleepless nights, after the application is moved to production.
If you Google, you will find thousands of links to good practices in exception handling. I am not going repeat all those. However, I will explain a few blunders found in exception handling.

One of the blunders done by the programmers is blank exception handler. See this










Whenever an exception is thrown from this code, it is simply suppressed. Remember, it is not far from now users come with a functionality is not working, and you are left clueless. No idea what happened. And if the users say, the problem happens randomly, you can be sure some thing wrong in exception handlers. What to do then? Simply replace the blank Exception handler with some code to catch and log the exception and move it to production. No other way. Then why don't you do it while you first code?

Here is the second blunder.








Well, here you are showing a user friendly message to the user. No need to worry about security aspects. Your password will not be shown to the user. Neither the database object names. But what happens when you need to fix this issue? What are you going to do with "Functionality not working"? Is it network issue or some one stopped the database? No way to find out.

Why programmers make such mistakes? After all, IT companies hire best people for their work. The answer is simple. The programmers are overconfident that their code will never break. So, they don't give much importance to the exception handlers. It is just done for the sake of coding standards.

Here is the third blunder.












(I have even seen Throw New Exception(ex.Message). But that is a different story). What's wrong here? You are just throwing the exception you got. The code will work the same way if the Exception handler were non existing. That's all? No.

Imagine, Inside SomeMethod another method is called (Say Method1). From this Method, another method, Method2, is called. Method1 and Method2 don't have exception handlers. Then one exception is thrown from Method2. It will be cascaded to the Exception handler in MyMethod (shown above). Then from catch block, a new exception is thrown. Remember the actual exception is happened in Method2, but in the logs you will find that it happened in MyMethod. Just remember this simple piece of misinformation can delay a problem fix for a few days, if it happened in an area you have least access, that is in production.

Here is the last one.










Wondering what's wrong here? I accept this the most intelligent one among the exception handlers listed so far. But here also we can improve.
  1. The message is shown to the user. Are you expecting the users to call you and inform you each time such a message is seen? Or will they keep a notepad to write down every strange messages they see? Oh! they have more important things to do. So, add a logging to the Exception Handlers. Use log4net, Enterprise Library or whatever you like, but add logging.
  2. What if you get a message "Object reference not set to an instance of an object."? No idea where it happened, among thousands of lines of code. You need to log the Exception.StackTrace as well. You need to log Exception.Message and Exception.StackTrace. There is an easy way. You can log Exception.ToString, which encompasses Message, StackTrace and even details of inner exceptions.
  3. Showing the actual exception message to the user may be risky at times. Who knows the message contains your credit card numbers or passwords? There is a high possibility error message contains database object names, which can make a hackers job easy. So, my advice is log the entire exception details and show the user a friendly message.
So, how can be the right one here? See below.


That's all for time being. Hope this will help some one to fix the production issues faster.

Thanks.


Tuesday, September 29, 2009

Active Threads in a Thread Pool (How I solved a problem)

I wanted to get number of threads active in the ThreadPool. Our client wants this to be logged. I thought it is walk on the park when I found the function ThreadPool.GetAvailableThreads. However, when I read more in MSDN, I found I was wrong.

The first question that came to my mind is why Microsoft is doing a Get and Set methods, instead of using a Property. More MS style was ThreadPool.AvailableThreds. Why not that way? Why the Java style getters and setters? I found the reason soon. Even though the name contains a Get, it returns void. then how can I get the available thread count? GetAvailableThreads has two byref parameters. See the declaration.

Public Shared Sub GetAvailableThreads ( _
ByRef workerThreads As Integer, _
ByRef completionPortThreads As Integer _
)
So we need to declare two int variables and pass it to this function. Then how the workerThreads and completionPortThreads differ?

Before proceeding further I found that GetAvailableThreads doesn't return number of active threads, rather it gives you number of threads that you can create (or queue) more. For example, if the Thread Pool maximum size is 10 and there are 3 active threads, then GetAvailableThreads returns 7 (10 - 3). That means you can queue seven more threads.

oh! That means more work for me. I need to find out maximum threads then I can subtract available threads from it, so that I will get active threads count. Good.

So, I found a GetMaxThreads method.

Public Shared Sub GetMaxThreads ( _
ByRef workerThreads As Integer, _
ByRef completionPortThreads As Integer _
)

Again, the previous question. What's completionPortThreads? I found the answer here. Here is the abstract.

"The worker thread is the one we all should be immediately familiar with. You are able to run tasks on a thread pool worker thread using ThreadPool.QueueUserWorkItem. There is currently a default maximum of 500 worker threads, but this may change in the future. You can also explicitly set the maximum number of worker threads using ThreadPool.SetMaxThreads.

The completion port threads are threads that are specifically designed to be used for asynchronous I/O operations. These threads wait on a single I/O completion port (IOCP). The .NET thread pool has exactly ONE IOCP. There is currently a default maximum of 1,000 IOCP threads, but this may change in the future. You can also explicitly set the maximum number of IOCP threads using ThreadPool.SetMaxThreads. You cannot set a maximum value less than the number of CPUs in the machine."

Ok, so I don't want completion port threads. I want only workerThreads. Now I have enough information to start coding.

The first thing I need is maximum thread counts.
' get maximum worker threads
Dim maxWorkerThreads As Integer
Dim maxIOThreads As Integer
ThreadPool.GetMaxThreads(maxWorkerThreads, maxIOThreads)
So, I have maximum threads count in the variable maxWorkerThreads. Now I need to find out available threads count same way.
' get remaining worker threads
Dim availableWorkerThreads As Integer
Dim availableIOThreads As Integer
ThreadPool.GetAvailableThreads(availableWorkerThreads, availableIOThreads)
Now, I have available threads count in availableWorkerThreads variable. The rest is easy.
activeThreads = maxWorkerThreads - availableWorkerThreads

I have deployed this to the QA server and about to leave office, when the testers stopped me. They have set MaxThreadCount to 1 (Oh! I did it earlier. I allowed them to set maximum threads through a configuration file). But again, it is showing 4 threads. I reviewed the code and imagined where the code can go wrong. I found the answer easily. The ThreadPool.SetMaxCount is not working. Microsoft did some thing bad here. But I thought it again. If I go to my manager and say Microsoft did some buggy code, she is not going to believe me. Is there any other way I can go wrong? I thought before saying any one Microsoft cheated us, better to read the documentation. I did, and immediately found the answer.

"You cannot set the number of worker threads or the number of I/O completion threads to a number smaller than the number of processors in the computer."

I immediately checked how many processors are there in the server. For my relief, I it was 4. With this MSDN URL, I can convince any one that it is not my fault.

That's how I solved a problem. Just posting in hope that it would help some one else.

Thanks,