Skip to content
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.

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

Close PtyProcess object instead of closing fd directly #24

Merged
merged 1 commit into from
Dec 10, 2015

Conversation

takluyver
Copy link
Member

@ellisonbg, I haven't got round to trying to reproduce the issue on your server, but I think I may have found one possible cause. If you have time, can you try installing from this branch and see if you can still reproduce the issue?

@ellisonbg
Copy link

Sure! Thanks for looking at this.

On Wed, Dec 9, 2015 at 9:35 AM, Thomas Kluyver notifications@github.com
wrote:

@ellisonbg https://github.com/ellisonbg, I haven't got round to trying
to reproduce the issue on your server, but I think I may have found one
possible cause. If you have time, can you try installing from this branch

and see if you can still reproduce the issue?

You can view, comment on, or merge this pull request online at:

#24
Commit Summary

  • Close PtyProcess object instead of closing fd directly

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#24.

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg
Copy link

Initial tests of this branch resolve the problem I am seeing. Two options:

  • We merge this now and I continue to test and monitor the situations in master.
  • I run this branch for a while to further confirm the fix.

I am fine with either. Thanks!

@takluyver
Copy link
Member Author

Thanks. I'm going to merge it, as with your testing, I'm fairly sure that it's an improvement. There's another change I wanted to make - dropping support for Tornado < 4 - and then I'll aim to do a new release.

takluyver added a commit that referenced this pull request Dec 10, 2015
Close PtyProcess object instead of closing fd directly
@takluyver takluyver merged commit e082abc into master Dec 10, 2015
@takluyver takluyver deleted the eof-close-pty-obj branch December 10, 2015 22:05
@ellisonbg
Copy link

Great, thanks!

On Thu, Dec 10, 2015 at 2:05 PM, Thomas Kluyver notifications@github.com
wrote:

Merged #24 #24.


Reply to this email directly or view it on GitHub
#24 (comment).

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

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.

None yet

2 participants