Disable lineLimitReader when handle BDAT data #139
Conversation
conn.go
Outdated
| @@ -753,6 +757,7 @@ func (c *Conn) handleBdat(arg string) { | |||
| } | |||
|
|
|||
| c.reset() | |||
| c.lineLimitReader.LineLimit = prevLineLimit | |||
There was a problem hiding this comment.
Instead of duplicating these statements, can we maybe use defer here?
prevLineLimit := c.lineLimitReader.LineLimit
c.lineLimitReader.LineLimit = 0
defer func() {
c.lineLimitReader.LineLimit = prevLineLimit
}()There was a problem hiding this comment.
I try it. But it not worked as expected.
There was a problem hiding this comment.
It just don't work. If i'm add defer and first chunk handle return error TooLongLine.
There was a problem hiding this comment.
Hm, what went wrong exactly?
I found problem. This code
defer func() {
c.lineLimitReader.LineLimit = prevLineLimit
}()
Run go routine and this can run defer and next handle at same time. I fix that with this code
recover := func() {
c.lineLimitReader.LineLimit = prevLineLimit
}
defer recover()
this fix problem same time running defer goroutine and next handleBdat
There was a problem hiding this comment.
I don't really understand why it doesn't work without the intermediary variable. It definitely should.
There was a problem hiding this comment.
It worked. But defer run in goroutine. This goroutine code
func() {
c.lineLimitReader.LineLimit = prevLineLimit
}()
And some time next handleBdat running before running defer. After this when we try copy chunk
io.Copy(c.bdatPipe, chunk)
Read return ErrTooLongLine
There was a problem hiding this comment.
defer doesn't run in a separate goroutine...
There was a problem hiding this comment.
Okey i'm research problem deeper and create new fix. First we should't restore limit before got BDAT LAST command. It's broke In some cases and return error ToooLongTime. For example read next BDAT command. I'm just put restore lineLimit in last condition and this fix problem.
There was a problem hiding this comment.
Oh you're right, if the BDAT transfer is still ongoing, we shouldn't restore the limit.
|
Can we also restore the limit in case |
|
Add recover on io.Copy fails |
We can fix this problem #81 (comment)
By add checkflag to disable limit checking on handle BDAT data