Skip to content

Commit

Permalink
bugfix: use same memory in ringbuffer
Browse files Browse the repository at this point in the history
ContainerIO' backend will read data into ringbuffer. But the data blocks
use the same memory so that the comming data will override the previous
one. Need to copy data before push into ringbuffer.

Signed-off-by: Wei Fu <[email protected]>
  • Loading branch information
fuweid committed Sep 21, 2018
1 parent 4f6d986 commit af36701
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 1 deletion.
11 changes: 10 additions & 1 deletion daemon/containerio/container_io.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,16 @@ func (cio *ContainerIO) converge(name, id string, in io.Reader) {
break
}

cover, err := cio.ring.Push(data[:n])
// FIXME(fuwei): In case that the data slice is reused by the writer,
// we should copy the data before we push it into the ringbuffer.
// The previous data shares the same address with the coming data.
// If we don't copy the data and the previous data isn't consumed by
// ringbuf pop action, the incoming data will override the previous data
// in the ringbuf.
copyData := make([]byte, n)
copy(copyData, data[:n])

cover, err := cio.ring.Push(copyData)
if err != nil {
break
}
Expand Down
59 changes: 59 additions & 0 deletions test/api_container_exec_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package main
import (
"bufio"
"bytes"
"crypto/rand"
"encoding/hex"
"io"
"net"
"strings"

"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/test/environment"
"github.com/alibaba/pouch/test/request"

Expand Down Expand Up @@ -46,6 +49,62 @@ func checkEchoSuccess(c *check.C, tty bool, conn net.Conn, br *bufio.Reader, exp
c.Assert(strings.TrimSpace(buf.String()), check.Equals, exp, check.Commentf("Expected %s, got %s", exp, buf.String()))
}

func (suite *APIContainerExecStartSuite) TestContainerExecWithLongInput(c *check.C) {
cname := "TestContainerExecWithLongInput"

CreateBusyboxContainerOk(c, cname)
defer DelContainerForceMultyTime(c, cname)
StartContainerOk(c, cname)

var execID string

// create exec
{
// echo the input
obj := map[string]interface{}{
"Cmd": []string{"cat"},
"AttachStderr": true,
"AttachStdout": true,
"AttachStdin": true,
"Tty": true, // avoid to use docker stdcopy
}

body := request.WithJSONBody(obj)
resp, err := request.Post("/containers/"+cname+"/exec", body)
c.Assert(err, check.IsNil)
CheckRespStatus(c, resp, 201)

var got types.ExecCreateResp
c.Assert(request.DecodeBody(&got, resp.Body), check.IsNil)
execID = got.ID
}

// start exec
{
obj := map[string]interface{}{
"Tty": true, // avoid to use docker stdcopy
}
resp, conn, br, err := request.Hijack("/exec/"+execID+"/start", request.WithJSONBody(obj))
c.Assert(err, check.IsNil)
CheckRespStatus(c, resp, 200)
defer conn.Close()

// input 256 chars
input := make([]byte, 256)
_, err = rand.Read(input)
c.Assert(err, check.IsNil)
content := hex.EncodeToString(input)

_, err = conn.Write([]byte(content + "\n"))
c.Assert(err, check.IsNil)

reader := bufio.NewReader(br)
got, _, err := reader.ReadLine()
c.Assert(err, check.IsNil)
c.Assert(string(got), check.Equals, content)
}
}

// TestContainerExecStartWithoutUpgrade tests start exec without upgrade which will return 200 OK.
func (suite *APIContainerExecStartSuite) TestContainerExecStartWithoutUpgrade(c *check.C) {
cname := "TestContainerCreateExecStartWithoutUpgrade"
Expand Down

0 comments on commit af36701

Please sign in to comment.