Skip to content

Commit

Permalink
optimize code about panic
Browse files Browse the repository at this point in the history
Signed-off-by: duxin40 <[email protected]>
  • Loading branch information
duxin40 committed Jan 4, 2025
1 parent ac75587 commit 4595abd
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ package tcp
*/
import "C"
import (
"runtime/debug"
"unsafe"

"github.com/envoyproxy/envoy/contrib/golang/common/go/api"
Expand All @@ -46,33 +45,15 @@ type httpRequest struct {
encodingState processState
}

// processState implements the FilterCallbacks interface.
type processState struct {
request *httpRequest
processState *C.processState
}

func (s *processState) RecoverPanic() {
if e := recover(); e != nil {
buf := debug.Stack()
api.LogErrorf("go side: golang http-tcp bridge: processState: panic serving: %v\n%s", e, buf)
// do nothing to retrun default value in origin func to continue normal status in c side.
// we DO NOT send local reply(panic) to c in order to avoid data race when upstream full-duplex.
}
}

func (r *httpRequest) pluginName() string {
return C.GoStringN(r.req.plugin_name.data, C.int(r.req.plugin_name.len))
}

// recover goroutine to stop Envoy process crashing when panic happens
func (r *httpRequest) recoverPanic() {
if e := recover(); e != nil {
buf := debug.Stack()
api.LogErrorf("o side: golang http-tcp bridge: httpRequest: panic serving: %v\n%s", e, buf)
}
}

func (r *httpRequest) GetRouteName() string {
// in upstream stage, route has been determined, so ignore error.
name, _ := cAPI.GetStringValue(unsafe.Pointer(r), ValueRouteName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ import "C"
import (
"errors"
"fmt"
"runtime/debug"
"sync"

"github.com/envoyproxy/envoy/contrib/golang/common/go/api"
)

var (
ErrDupRequestKey = errors.New("dup request key")

ErrorInfoForPanic = "error happened in golang http-tcp bridge\r\n"
)

var Requests = &requestMap{}
Expand Down Expand Up @@ -118,10 +121,8 @@ func getRequest(r *C.httpRequest) *httpRequest {
}

//export envoyGoHttpTcpBridgeOnEncodeHeader
func envoyGoHttpTcpBridgeOnEncodeHeader(s *C.processState, endStream, headerNum, headerBytes, buffer, length uint64) uint64 {
func envoyGoHttpTcpBridgeOnEncodeHeader(s *C.processState, endStream, headerNum, headerBytes, buffer, length uint64) (status uint64) {
state := getOrCreateState(s)
defer state.RecoverPanic()

req := state.request
buf := &httpBuffer{
state: state,
Expand All @@ -139,14 +140,21 @@ func envoyGoHttpTcpBridgeOnEncodeHeader(s *C.processState, endStream, headerNum,
},
},
}

defer func() {
if e := recover(); e != nil {
api.LogErrorf("go side: golang http-tcp bridge: encodeHeader: panic serving: %v\n%s", e, debug.Stack())
status = uint64(api.HttpTcpBridgeContinue)
buf.SetString(ErrorInfoForPanic)
}
}()

return uint64(filter.EncodeHeaders(header, buf, endStream == uint64(api.EndStream)))
}

//export envoyGoHttpTcpBridgeOnEncodeData
func envoyGoHttpTcpBridgeOnEncodeData(s *C.processState, endStream, buffer, length uint64) uint64 {
func envoyGoHttpTcpBridgeOnEncodeData(s *C.processState, endStream, buffer, length uint64) (status uint64) {
state := getOrCreateState(s)
defer state.RecoverPanic()

req := state.request
buf := &httpBuffer{
state: state,
Expand All @@ -156,15 +164,21 @@ func envoyGoHttpTcpBridgeOnEncodeData(s *C.processState, endStream, buffer, leng

filter := req.httpTcpBridge

defer func() {
if e := recover(); e != nil {
api.LogErrorf("go side: golang http-tcp bridge: encodeData: panic serving: %v\n%s", e, debug.Stack())
status = uint64(api.HttpTcpBridgeContinue)
buf.SetString(ErrorInfoForPanic)
}
}()

return uint64(filter.EncodeData(buf, endStream == uint64(api.EndStream)))
}

//export envoyGoHttpTcpBridgeOnUpstreamData
func envoyGoHttpTcpBridgeOnUpstreamData(s *C.processState, endStream, headerNum, headerBytes, buffer, length uint64) uint64 {
func envoyGoHttpTcpBridgeOnUpstreamData(s *C.processState, endStream, headerNum, headerBytes, buffer, length uint64) (status uint64) {

state := getOrCreateState(s)
defer state.RecoverPanic()

req := state.request
buf := &httpBuffer{
state: state,
Expand All @@ -183,14 +197,27 @@ func envoyGoHttpTcpBridgeOnUpstreamData(s *C.processState, endStream, headerNum,
},
}

defer func() {
if e := recover(); e != nil {
api.LogErrorf("go side: golang http-tcp bridge: onUpstreamData: panic serving: %v\n%s", e, debug.Stack())
status = uint64(api.HttpTcpBridgeEndStream)
buf.SetString(ErrorInfoForPanic)
}
}()

return uint64(filter.OnUpstreamData(header, buf, endStream == uint64(api.EndStream)))
}

//export envoyGoHttpTcpBridgeOnDestroy
func envoyGoHttpTcpBridgeOnDestroy(r *C.httpRequest) {
req := getRequest(r)
// do nothing even when get panic, since filter is already destroying.
defer req.recoverPanic()
defer func() {
if e := recover(); e != nil {
buf := debug.Stack()
api.LogErrorf("go side: golang http-tcp bridge: httpRequest: panic serving: %v\n%s", e, buf)
}
}()

f := req.httpTcpBridge
f.OnDestroy()
Expand Down
9 changes: 4 additions & 5 deletions contrib/golang/upstreams/http/tcp/source/upstream_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ HttpTcpBridge::HttpTcpBridge(Router::UpstreamToDownstream* upstream_request,
plugin_name.data = config->pluginName().data();
plugin_name.len = config->pluginName().length();

route_name_ = upstream_request_->route().virtualHost().routeConfig().name();

upstream_conn_data_->connection().enableHalfClose(true);
upstream_conn_data_->addUpstreamCallbacks(*this);
}
Expand Down Expand Up @@ -314,6 +312,7 @@ void HttpTcpBridge::encodeDataGo(Buffer::Instance& data, bool end_stream) {
s, end_stream ? 1 : 0, reinterpret_cast<uint64_t>(&data), data.length());

encoding_state_.handleDataGolangStatus(static_cast<HttpTcpBridgeStatus>(go_status), end_stream);
ENVOY_LOG(debug, "golang http-tcp bridge encodeDataGo, state: {}", encoding_state_.stateStr());

if (encoding_state_.filterState() == FilterState::Done ||
encoding_state_.filterState() == FilterState::WaitingData) {
Expand All @@ -322,12 +321,12 @@ void HttpTcpBridge::encodeDataGo(Buffer::Instance& data, bool end_stream) {
}

void HttpTcpBridge::sendDataToDownstream(Buffer::Instance& data, bool end_stream) {
// we can send resp headers only one time.
if (!already_send_resp_headers_) {
ENVOY_LOG(debug, "golang http-tcp bridge send resp headers to downstream. end_stream: {}",
end_stream);
// we can send resp headers only one time.
// if go side not set status, c++ side set default status
if (!decoding_state_.resp_headers->Status()) {
// if go side not set status, c++ side set default status
decoding_state_.resp_headers->setStatus(static_cast<uint64_t>(HttpStatusCode::Success));
}
upstream_request_->decodeHeaders(std::move(decoding_state_.resp_headers), false);
Expand Down Expand Up @@ -528,7 +527,7 @@ CAPIStatus HttpTcpBridge::getStringValue(int id, uint64_t* value_data, int* valu
// it on the Go side.
switch (static_cast<EnvoyValue>(id)) {
case EnvoyValue::RouteName:
str_value_ = route_name_;
str_value_ = upstream_request_->route().virtualHost().routeConfig().name();
break;
case EnvoyValue::ClusterName: {
str_value_ = route_entry_->clusterName();
Expand Down
4 changes: 0 additions & 4 deletions contrib/golang/upstreams/http/tcp/source/upstream_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,6 @@ class HttpTcpBridge : public Router::GenericUpstream,

const Router::RouteEntry* route_entry_;

// cache routeName for getStringValue, since
// upstream_request_->route().virtualHost().routeConfig().name() is not concurrent safe.
std::string route_name_;

private:
Router::UpstreamToDownstream* upstream_request_;
Envoy::Tcp::ConnectionPool::ConnectionDataPtr upstream_conn_data_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type httpTcpBridge struct {
*
*/
func (f *httpTcpBridge) EncodeHeaders(headerMap api.RequestHeaderMap, dataToUpstream api.BufferInstance, endOfStream bool) api.HttpTcpBridgeStatus {
api.LogInfof("[EncodeHeaders] come, endStream: %v", endOfStream)
// =========== step 1: get dubbo method and interface from http header =========== //
dubboMethod, _ := headerMap.Get("dubbo_method")
dubboInterface, _ := headerMap.Get("dubbo_interface")
Expand Down

0 comments on commit 4595abd

Please sign in to comment.