Skip to content

fix file descriptor leak problem & enable keepalive by default#191

Open
plpan wants to merge 1 commit intoparnurzeal:developfrom
ppltools:develop
Open

fix file descriptor leak problem & enable keepalive by default#191
plpan wants to merge 1 commit intoparnurzeal:developfrom
ppltools:develop

Conversation

@plpan
Copy link
Copy Markdown

@plpan plpan commented Jun 4, 2018

No description provided.

@pkopac
Copy link
Copy Markdown
Collaborator

pkopac commented Nov 9, 2018

@QuentinPerez: IDK about the keep-alive, but the resp.Body.Close() is certainly not correct in current code; it only gets invoked on the NopCloser, which... does nothing. So, the Reader from http stays open forever.

@pkopac pkopac mentioned this pull request Nov 9, 2018
@plpan
Copy link
Copy Markdown
Author

plpan commented Nov 15, 2018

@pkopac When the resp.Body is closed, we can fix leak by reusing the obj of SuperAgent. We have proved it works.

@pkopac
Copy link
Copy Markdown
Collaborator

pkopac commented Nov 15, 2018

@plpan: thanks, makes sense... but, if you don't reuse all instances (eg. we don't in our code), then it leaks. So I still consider this a bug.

@plpan
Copy link
Copy Markdown
Author

plpan commented Nov 16, 2018

@pkopac Yeal, you're right. We should disable it by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants