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

improve TTY write buffering #6375

Closed
tanmaykm opened this issue Apr 2, 2014 · 15 comments
Closed

improve TTY write buffering #6375

tanmaykm opened this issue Apr 2, 2014 · 15 comments
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Milestone

Comments

@tanmaykm
Copy link
Member

tanmaykm commented Apr 2, 2014

Noticed initially in some code that printed lot of debug messages on to stdout.

The following results in occasional garbage being printed on the terminal (tried on macos and ubuntu):

for i in 1:100000 println(randstring(100)); end

Seems to work fine if printed on to a file instead.

@rwgardner
Copy link

I am getting the same issue although I would actually change the title to include "memory corruption" or something.

for i = 1:100000
   a=[1 2 3 4]
   println("a: ", a)
end

destroys my runtime and terminal.

@bicycle1885
Copy link
Member

I also encountered the issue.
Using git bisect, I've found that 1aed903 seems to be the first bad commit.

@Keno
Copy link
Member

Keno commented Apr 5, 2014

Notice my comment
Wait a minute. Does libuv do non-blocking writes to TTYs now? We make the assumption that writes to TTYs are thread-blocking in a number of places.. That issue isn't just pedagogical ;)

@JeffBezanson JeffBezanson added this to the 0.3 milestone Apr 5, 2014
@JeffBezanson
Copy link
Member

This is release-blocking.

@nolta
Copy link
Member

nolta commented Apr 5, 2014

Removing the TTY write methods from base/stream.jl appears to fix this problem. But then the test output is screwed up,

    JULIA test/all
    From worker 9:      From worker 3:      From worker 4:      From worker 7:      From worker 5:      From worker 6:       * collections
    From worker 2:       * linalg2
     * linalg3
     * numbers
     * core
     * keywordargs
     * linalg1
    From worker 8:       * strings
    From worker 6:       * hashing
    From worker 6:       * remote
    From worker 6:       * iobuffer
...

This, in turn, seems to be fixed by

diff --git a/base/multi.jl b/base/multi.jl
index 5eafe68..14cecab 100644
--- a/base/multi.jl
+++ b/base/multi.jl
@@ -1035,7 +1035,7 @@ function create_worker(privhost, port, pubhost, stream, config, manage)
             @async begin
                 while !eof(stream)
                     line = readline(stream)
-                    print("\tFrom worker $(wrker.id):\t",bytestring(line))
+                    print("\tFrom worker $(wrker.id):\t $line")
                 end
             end
         end

@carlobaldassi
Copy link
Member

@nolta the test output screwup is very similar to #5770 (which weirdly I seemed to be the only one having), and indeed your fix works in my case, so I pushed it, thanks.

@JeffBezanson
Copy link
Member

@loladiro could you comment on @nolta 's above suggestion?

@Keno
Copy link
Member

Keno commented Apr 8, 2014

Yeah, that would work fine. I haven't gotten a chance yet to look at what the hell libuv did to ttys, but that is certainly the right change if they are now not thread-blocking.

@staticfloat
Copy link
Member

This change has already been made: aadabde

@Keno
Copy link
Member

Keno commented Apr 8, 2014

I think @JeffBezanson was referring to removing the TTY methods, which I don't think has happened yet.

@staticfloat
Copy link
Member

Ah, gotcha.

On Tue, Apr 8, 2014 at 3:52 PM, Keno Fischer [email protected]:

I think @JeffBezanson https://github.com/JeffBezanson was referring to
removing the TTY methods, which I don't think has happened yet.

Reply to this email directly or view it on GitHubhttps://github.com//issues/6375#issuecomment-39911442
.

@nolta nolta closed this as completed in 3aa965b Apr 9, 2014
@JeffBezanson JeffBezanson reopened this Apr 15, 2014
@JeffBezanson
Copy link
Member

Removing the TTY methods is ok, but the fix in multi.jl is not general --- other code that uses print or println does not get line buffering, so its output is still not what one would prefer.

@Keno
Copy link
Member

Keno commented Apr 20, 2014

The action item here is to make use of libuvs capability to write multiple buffers in one write request and block on that. Might get to it tomorrow.

@vtjnash vtjnash added feature and removed bug labels Apr 26, 2014
@vtjnash vtjnash modified the milestones: 0.4, 0.3 Apr 26, 2014
@vtjnash
Copy link
Member

vtjnash commented Apr 26, 2014

moving this to 0.4, since the bug is fixed

@vtjnash vtjnash closed this as completed Apr 26, 2014
@vtjnash vtjnash reopened this Apr 26, 2014
@JeffBezanson JeffBezanson changed the title Corrupted output while printing to a terminal improve TTY write buffering Jul 27, 2014
@vtjnash
Copy link
Member

vtjnash commented Aug 24, 2015

closing this since the original bug was fixed, the followup issue has been improved by adding locking to streams during IO, and further improvement is covered by #3887

@vtjnash vtjnash closed this as completed Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

No branches or pull requests

9 participants