8

The users of our application download an attachment at least once in two seconds during the day.

Previous Scenario:

We were using Response.End() to abort the connection to the client after the user downloads an attachment. As we were having performance issues, we started logging exceptions and one of the most repeated one was the thread abort exception. Since we are getting the attachment from a web service, We have to do some clean up and we had the clean up in try-catch-finally block. After some research, I have understood that any code after Response.End() will not be executed even if it is in finally block. Is that right?

Current Scenario:

I've read the thread in stack overflow about Response.End() being harmful and it needs to be used only when it is really needed, so I decided to use HttpContext....CompleteRequest() instead. With this code, the clean up needed is done, but the html that is rendered is being appended to the attachment downloaded. I tried overriding the Render and RaisePostBackEvent suggested in the same article but the problem still persists. Any idea on how to resolve this issue would be helpful.

Code:

HttpContext.Current.Response.Clear();

Response.ClearContent();

Response.ClearHeaders();

Response.AddHeader("Content-Disposition", "attachment; filename=" +   
filename);
Response.AddHeader("Content-Length", fileContent.Length.ToString());
Response.ContentType = "application/octet-stream";
Response.BinaryWrite(fileContent);
Response.Flush();
John Saunders
  • 160,644
  • 26
  • 247
  • 397
user1396468
  • 83
  • 1
  • 1
  • 4

2 Answers2

7

Response.End internally throws the ThreadAbortException to kill the request - if you need to do some kind of cleanup, this needs to be done before the call to Response.End is made.

Response.Redirect and Response.End do not interact well with try / catch blocks. So in your situation, you should do all your logic writing to the response stream in your try / catch, then simply call Response.End after your finally block.

Tejs
  • 40,736
  • 10
  • 68
  • 86
  • Thanks for the reply. I understand that will work, but how can I get rid of the ThreadAbortException, its filling up my log files and probably hogging some memory on the server too. Is there a way I can do this with out the Response.End(). edit: Also, if I use Response.End() after my finally block, I am running in to the same issue of rendered html being appended to the document downloaded. Anyway I can resolve that. Thank You for the reply again. – user1396468 May 15 '12 at 15:47
  • 1
    Stop wrapping the `Response.End` in your try / catch. `Response.End` uses the `ThreadAbortException` to end the response. That's just how it works. You're basically catching the framework's exception. – Tejs May 15 '12 at 15:48
  • Ok. I will not wrap Response.End() in a try.. catch. Thanks for pointing that out. can you reply to my edit, I just tried the code you suggested, and I have the html problem again. I really appreciate your responses. thank you. – user1396468 May 15 '12 at 15:52
  • I've edited the post with the code, this is the code that is in the try..catch block and I've placed the response.end() after finally. Also if it helps, I am redirecting to an empty download page, the code posted above resides in Page_load of that download page which will show up the open/save dialog. Thank You. – user1396468 May 15 '12 at 16:01
  • finally blocks are executed in any case. They are never skipped. There is no way to kill a thread. – usr May 15 '12 at 16:05
  • tejs.. ur answer works, I was wrong in checking the content. Thank you. I am marking your reply as answer. – user1396468 May 15 '12 at 16:14
2

Old Q, and not sure if it helps, but I do essentially the same as your example when creating PDF files to deliver to the browser. However, at the end of processing, I have the following:

    Response.OutputStream.Flush()
    Response.OutputStream.Close()
    Response.End()

The addition of .Close() basically. Seems to work fine in production.

Ed: Just found this too, mentioning Close(): Is Response.End() considered harmful?

Another approach to your problem is simply overriding Page_PreRender() and putting the content delivery code in there (which kind of makes functional sense). Then you definitely won't get any unwanted HTML, and there should be no need for Response.End() at all.

Community
  • 1
  • 1
ingredient_15939
  • 3,022
  • 7
  • 35
  • 55