Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Add destructor to RequestStream #2430

Merged
merged 1 commit into from
Apr 26, 2016
Merged

Add destructor to RequestStream #2430

merged 1 commit into from
Apr 26, 2016

Conversation

danbarua
Copy link
Contributor

@danbarua danbarua commented Apr 26, 2016

Prerequisites

  • [ x] I have written a descriptive pull-request title
  • [ x] I have verified that there are no overlapping pull-requests open
  • [x ] I have verified made sure that I am following the Nancy code style guidelines
  • [ x] I have provided test coverage for your change (where applicable)

Description

Given a large (>85kb) request
When .Dispose() is not explicitly called (exception?)
Then we leak temp files

Reports from ops that it is leaking one temp file for every upload that throws an exception.

Related to #2159

@danbarua danbarua force-pushed the master branch 2 times, most recently from 5afcc63 to a9b6fa1 Compare April 26, 2016 15:19
@@ -195,12 +200,14 @@ public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, As
{
return this.stream.BeginWrite(buffer, offset, count, callback, state);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😞

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grrr CodeLens

…emp files if .Dispose() not explicitly called.
@khellang khellang added this to the 2.0-beta milestone Apr 26, 2016
@khellang khellang added the Bug label Apr 26, 2016
@khellang khellang merged commit 244aad3 into NancyFx:master Apr 26, 2016
danbarua added a commit to danbarua/Nancy that referenced this pull request Apr 26, 2016
…emp files if .Dispose() not explicitly called. (NancyFx#2430)

# Conflicts:
#	src/Nancy/IO/RequestStream.cs
@xt0rted xt0rted mentioned this pull request Jul 3, 2017
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants