Skip to content

Commit

Permalink
fix(client): keep ref to resp to avoid conn closed
Browse files Browse the repository at this point in the history
  • Loading branch information
xiaost committed Nov 27, 2024
1 parent 193c8be commit 2a38ed2
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 76 deletions.
48 changes: 19 additions & 29 deletions .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
@@ -1,46 +1,36 @@
name: Pull Request Check

on: [pull_request]
on: [ pull_request ]

jobs:
compliant:
runs-on: self-hosted
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Check License Header
uses: apache/skywalking-eyes@header@v0.4.0
uses: apache/skywalking-eyes/header@v0.4.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: typos-action
- name: Check Spell
uses: crate-ci/typos@master

staticcheck:
runs-on: self-hosted
golangci-lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- uses: actions/checkout@v4
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version: 1.16

- uses: actions/cache@v3
uses: actions/setup-go@v5
with:
path: ~/go/pkg/mod
key: reviewdog-${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
reviewdog-${{ runner.os }}-go-
- uses: reviewdog/action-staticcheck@v1
go-version: stable
# for self-hosted, the cache path is shared across projects
# and it works well without the cache of github actions
# Enable it if we're going to use Github only
cache: true

- name: Golangci Lint
# https://golangci-lint.run/
uses: golangci/golangci-lint-action@v6
with:
github_token: ${{ secrets.github_token }}
# Change reviewdog reporter if you need [github-pr-check,github-check,github-pr-review].
reporter: github-pr-review
# Report all results.
filter_mode: nofilter
# Exit with 1 when it find at least one finding.
fail_on_error: true
# Set staticcheck flags
staticcheck_flags: -checks=inherit,-SA1029,-SA6002
version: latest
25 changes: 0 additions & 25 deletions .github/workflows/release-check.yml

This file was deleted.

41 changes: 21 additions & 20 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
name: Tests

on: [push, pull_request]
on: [ push, pull_request ]

jobs:
lint-and-ut:
runs-on: self-hosted
benchmark:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- uses: actions/checkout@v4
- name: Set up Go
uses: actions/setup-go@v3
with:
go-version: 1.18

- uses: actions/cache@v3
uses: actions/setup-go@v5
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-
go-version: stable

- name: Lint
run: |
go vet -stdmethods=false $(go list ./...)
go install mvdan.cc/gofumpt@v0.2.0
test -z "$(gofumpt -l -extra .)"
- name: Benchmark
run: go test -bench=. -benchmem ./...

uinttest:
strategy:
matrix:
go: [ "1.18", "1.19", "1.20", "1.21", "1.22", "1.23" ]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go }}
cache: true # set false for self-hosted
- name: Unit Test
run: go test -race -covermode=atomic -coverprofile=coverage.out ./...
run: go test -race ./...
10 changes: 10 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
linters: # https://golangci-lint.run/usage/linters/
disable-all: true
enable:
# - errcheck # can not skip _test.go ?
- gosimple
- govet
- ineffassign
- staticcheck
- unused
- unconvert
1 change: 1 addition & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,6 @@ func (p *ClientUpgrader) UpgradeResponse(req *protocol.Request, resp *protocol.R
conn.newCompressionWriter = compressNoContextTakeover
conn.newDecompressionReader = decompressNoContextTakeover
}
conn.resp = resp
return conn, nil
}
4 changes: 4 additions & 0 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ type Conn struct {

readDecompress bool // whether last read frame had RSV1 set
newDecompressionReader func(io.Reader) io.ReadCloser

// keep reference to the resp to make sure the underyling conn will not be closed.
// see: https://github.com/cloudwego/hertz/pull/1214 for the details.
resp interface{} // *protocol.Response
}

func newConn(conn net.Conn, isServer bool, readBufferSize, writeBufferSize int, writeBufferPool BufferPool, br *bufio.Reader, writeBuf []byte) *Conn {
Expand Down
2 changes: 1 addition & 1 deletion conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestFraming(t *testing.T) {
return w.Write(writeBuf[:n])
}},
{"string", func(w io.Writer, n int) (int, error) {
return io.WriteString(w, string(writeBuf[:n]))
return io.WriteString(w, string(writeBuf[:n])) // nolint: staticcheck
}},
}

Expand Down
4 changes: 3 additions & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,9 @@ func (u *HertzUpgrader) Upgrade(ctx *app.RequestContext, handler HertzHandler) e
handler(c)

writeBuf = writeBuf[0:0]
poolWriteBuffer.Put(writeBuf)

// FIXME: argument should be pointer-like to avoid allocations (staticcheck)
poolWriteBuffer.Put(writeBuf) // nolint: staticcheck
})

return nil
Expand Down

0 comments on commit 2a38ed2

Please sign in to comment.