Skip to content

AHC-78: OOM with multi parts #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

AHC-78: OOM with multi parts #12

wants to merge 2 commits into from

Conversation

gailh
Copy link
Contributor

@gailh gailh commented May 9, 2011

AHC

@jfarcand
Copy link
Contributor

jfarcand commented May 9, 2011

Thanks!! Looking and running our internal test

@gailh
Copy link
Contributor Author

gailh commented May 9, 2011

Sounds good. You may need to make some changes. I just tested it with my
local test.

Gail

On Mon, May 9, 2011 at 10:18 AM, jfarcand <
reply@reply.github.com>wrote:

Thanks!! Looking and running our internal test

Reply to this email directly or view it on GitHub:
https://github.com/sonatype/async-http-client/pull/12#issuecomment-1124882

@jfarcand
Copy link
Contributor

Hi, can you do a pull with the complete files? Right now I do have issue merging this pull with the previous one. If you can do one will full changes, I will start from there. Thanks!

@jfarcand jfarcand closed this May 10, 2011
@gailh
Copy link
Contributor Author

gailh commented May 10, 2011

I'm not sure how to do that. I only changed one file the MultiPartbody.

Gail

On Tue, May 10, 2011 at 5:51 AM, jfarcand <
reply@reply.github.com>wrote:

Hi, can you do a pull with the complete files? Right now I do have issue
merging this pull with the previous one. If you can do one will full
changes, I will start from there. Thanks!

Reply to this email directly or view it on GitHub:
https://github.com/sonatype/async-http-client/pull/12#issuecomment-1130675

@jfarcand
Copy link
Contributor

You need to include all the changes you did. When I do the pull I don't get the change in NettyAsycnHttpProvider so the old OOM occurs.

@gailh
Copy link
Contributor Author

gailh commented May 10, 2011

I for this fix I didn't make any changes in there. Only on the first change
I made the changes there.

Gail

On Tue, May 10, 2011 at 7:42 AM, jfarcand <
reply@reply.github.com>wrote:

You need to include all the changes you did. When I do the pull I don't get
the change in NettyAsycnHttpProvider so the old OOM occurs.

Reply to this email directly or view it on GitHub:
https://github.com/sonatype/async-http-client/pull/12#issuecomment-1131285

@jfarcand
Copy link
Contributor

OK let me take a look again.

@jfarcand jfarcand reopened this May 10, 2011
@jfarcand
Copy link
Contributor

Gail, just to be sure before I work on it. Just applying this pull will not fix the OOM, righ?

@gailh
Copy link
Contributor Author

gailh commented May 11, 2011

The issue was the read(ByteBuffer) wasn't implemented. That seemed to be
where it was failing. So i'm not sure if this will fix the OOM. I just
changed it so that it implements read(ByteBuffer).

Gail

On Wed, May 11, 2011 at 11:28 AM, jfarcand <
reply@reply.github.com>wrote:

Gail, just to be sure before I work on it. Just applying this pull will not
fix the OOM, righ?

Reply to this email directly or view it on GitHub:
https://github.com/sonatype/async-http-client/pull/12#issuecomment-1142108

@jfarcand
Copy link
Contributor

Ok, apology for being lost but I need to have all the classes in order to properly test/integrate it. Right now I'm not sure I do have all your changes...so can you send or pull all the changes you did to the workspace? You must have made some change in NettyAsyncHttpProvider as well, right? Since you have tested your fix for the initial issue, I want to start from there. Thanks for your patience!

@gailh
Copy link
Contributor Author

gailh commented May 12, 2011

I think something is wrong with the way I pushed the commits. They are
still lited in my client as pushable. And I can't do a pull request. So I
don't think the changes are getting in.

Any ideas?

It says the pushes pushed, but I can't request a pull? And they are still
listed as pushable commits.

Thanks!

Gail

On Thu, May 12, 2011 at 5:42 AM, jfarcand <
reply@reply.github.com>wrote:

Ok, apology for being lost but I need to have all the classes in order to
properly test/integrate it. Right now I'm not sure I do have all your
changes...so can you send or pull all the changes you did to the workspace?
You must have made some change in NettyAsyncHttpProvider as well, right?
Since you have tested your fix for the initial issue, I want to start from
there. Thanks for your patience!

Reply to this email directly or view it on GitHub:
https://github.com/sonatype/async-http-client/pull/12#issuecomment-1146190

@jfarcand
Copy link
Contributor

Apology for the delay. Can you zip your source code and send it to me jfarcand at apache dot org. I will go from there.

Thanks!

@jfarcand
Copy link
Contributor

FYi, with your latest code I do get

T 127.0.0.1:59713 -> 127.0.0.1:59720 [AP]
HTTP/1.1 500 Processing of multipart/form-data request failed. Stream ended unexpectedly.
Content-Type: text/html;charset=ISO-8859-1.
Cache-Control: must-revalidate,no-cache,no-store.
Content-Length: 1519.
Server: Jetty(7.0.0.v20091005).
.

<title>Error 500 Processing of multipart/form-data request failed. Stream ended unexpectedly</title>

HTTP ERROR 500

Problem accessing /secure/AttachFile.jspa. Reason: 

    Processing of multipart/form-data request failed. Stream ended unexpectedly

Powered by Jetty://

@jfarcand
Copy link
Contributor

Fixed/integrated

https://github.com/sonatype/async-http-client/commit/14faa11c3b8678ccc650cdd46966335a834cc3a6

@jfarcand jfarcand closed this May 31, 2011
cs-workco pushed a commit to cs-workco/async-http-client that referenced this pull request Apr 13, 2023
@tomerd
fix formatting (AsyncHttpClient#12)
4acd312 
motivation: use consistent formatting

changes:
* run `swiftformat --self insert --patternlet inline --stripunusedargs unnamed-only --comments ignore` which is the standard formatting we use
* update generate_linux_tests to follow same formatting rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gailh @jfarcand