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 724fe1a
Show file tree
Hide file tree
Showing 9 changed files with 91 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
17 changes: 17 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright 2024 CloudWeGo Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package websocket

import (
Expand Down Expand Up @@ -86,5 +102,6 @@ func (p *ClientUpgrader) UpgradeResponse(req *protocol.Request, resp *protocol.R
conn.newCompressionWriter = compressNoContextTakeover
conn.newDecompressionReader = decompressNoContextTakeover
}
conn.resp = resp
return conn, nil
}
16 changes: 16 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
* Copyright 2024 CloudWeGo Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package websocket

import (
Expand Down
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 724fe1a

Please sign in to comment.