Skip to content

Commit

Permalink
use "select" to reduce race operations
Browse files Browse the repository at this point in the history
The current race operations are all guarded with "try", they won't trigger any bug

resolve #93
  • Loading branch information
ysmood committed Jul 7, 2020
1 parent 32278e5 commit 2963ef2
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 56 deletions.
55 changes: 24 additions & 31 deletions lib/cdp/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ import (

// Client is a devtools protocol connection instance.
type Client struct {
ctx context.Context
ctxCancel func()
ctxCancelErr error
ctx context.Context
ctxCancel func()

wsURL string
header http.Header
Expand Down Expand Up @@ -159,26 +158,21 @@ func (cdp *Client) Call(ctx context.Context, sessionID, method string, params in
kit.E(err)

callback := make(chan *response)
defer close(callback)

cdp.callbacks.Store(req.ID, callback)
defer cdp.callbacks.Delete(req.ID)

e := kit.Try(func() {
cdp.chReq <- data
})
if err, ok := e.(error); ok {
if cdp.ctxCancelErr != nil {
return nil, cdp.ctxCancelErr
}
return nil, err
select {
case <-cdp.ctx.Done():
return nil, cdp.ctx.Err()

case <-ctx.Done():
return nil, ctx.Err()
case cdp.chReq <- data:
}

select {
case <-cdp.ctx.Done():
if cdp.ctxCancelErr != nil {
return nil, cdp.ctxCancelErr
}
return nil, cdp.ctx.Err()

case <-ctx.Done():
Expand All @@ -205,12 +199,6 @@ type requestMsg struct {

// consume messages from client and browser
func (cdp *Client) consumeMsg() {
defer func() {
close(cdp.chReq)
close(cdp.chRes)
close(cdp.chEvent)
}()

for {
select {
case <-cdp.ctx.Done():
Expand All @@ -234,9 +222,11 @@ func (cdp *Client) consumeMsg() {

callback, has := cdp.callbacks.Load(res.ID)
if has {
_ = kit.Try(func() {
callback.(chan *response) <- res
})
select {
case <-cdp.ctx.Done():
return
case callback.(chan *response) <- res:
}
}
}
}
Expand All @@ -262,23 +252,26 @@ func (cdp *Client) readMsgFromBrowser() {
err := json.Unmarshal(data, &res)
kit.E(err)
cdp.debugLog(&res)
_ = kit.Try(func() {
cdp.chRes <- &res
})
select {
case <-cdp.ctx.Done():
return
case cdp.chRes <- &res:
}
} else {
var evt Event
err := json.Unmarshal(data, &evt)
kit.E(err)
cdp.debugLog(&evt)
_ = kit.Try(func() {
cdp.chEvent <- &evt
})
select {
case <-cdp.ctx.Done():
return
case cdp.chEvent <- &evt:
}
}
}
}

func (cdp *Client) close(err error) {
cdp.debugLog(err)
cdp.ctxCancelErr = err
cdp.ctxCancel()
}
2 changes: 1 addition & 1 deletion lib/cdp/main_private_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ func TestWriteError(t *testing.T) {
cdp.wsConn = &wsWriteErrConn{}
go cdp.consumeMsg()
_, err := cdp.Call(context.Background(), "", "", nil)
assert.EqualError(t, err, "err")
assert.EqualError(t, err, "context canceled")
}
2 changes: 1 addition & 1 deletion lib/cdp/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,5 @@ func TestCrash(t *testing.T) {
"expression": `new Promise(() => {})`,
"awaitPromise": true,
})
assert.Regexp(t, `websocket: close 1006 \(abnormal closure\)|forcibly closed by the remote host`, err.Error())
assert.Regexp(t, `context canceled`, err.Error())
}
53 changes: 30 additions & 23 deletions lib/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@ import (

// Launcher is a helper to launch browser binary smartly
type Launcher struct {
ctx context.Context
bin string
url string
log func(string)
Flags map[string][]string `json:"flags"`
output chan string
pid int
exit chan kit.Nil
reap bool
ctx context.Context
ctxCancel func()
bin string
url string
log func(string)
Flags map[string][]string `json:"flags"`
output chan string
pid int
exit chan kit.Nil
reap bool
}

// New returns the default arguments to start browser.
Expand Down Expand Up @@ -85,22 +86,26 @@ func New() *Launcher {
defaultFlags["no-sandbox"] = nil
}

ctx, cancel := context.WithCancel(context.Background())
return &Launcher{
ctx: context.Background(),
Flags: defaultFlags,
output: make(chan string),
exit: make(chan kit.Nil),
bin: defaults.Bin,
reap: true,
ctx: ctx,
ctxCancel: cancel,
Flags: defaultFlags,
output: make(chan string),
exit: make(chan kit.Nil),
bin: defaults.Bin,
reap: true,
}
}

// NewUserMode is a preset to enable reusing current user data. Useful for automation of personal browser.
// If you see any error, it may because you can't launch debug port for existing browser, the solution is to
// completely close the running browser. Unfortunately, there's no API for rod to tell it automatically yet.
func NewUserMode() *Launcher {
ctx, cancel := context.WithCancel(context.Background())
return &Launcher{
ctx: context.Background(),
ctx: ctx,
ctxCancel: cancel,
Flags: map[string][]string{
"remote-debugging-port": {"37712"},
},
Expand All @@ -111,7 +116,9 @@ func NewUserMode() *Launcher {

// Context set the context
func (l *Launcher) Context(ctx context.Context) *Launcher {
ctx, cancel := context.WithCancel(ctx)
l.ctx = ctx
l.ctxCancel = cancel
return l
}

Expand Down Expand Up @@ -240,6 +247,8 @@ func (l *Launcher) LaunchE() (string, error) {
runReaper()
}

defer l.ctxCancel()

bin := l.bin
if bin == "" {
var err error
Expand Down Expand Up @@ -336,20 +345,18 @@ func (l *Launcher) read(reader io.Reader) {
if l.log != nil {
l.log(str)
}
_ = kit.Try(func() {
l.output <- str
})
select {
case <-l.ctx.Done():
return
case l.output <- str:
}
}
}

// ReadURL from browser stderr
func (l *Launcher) getURL() (string, error) {
out := ""

defer func() {
close(l.output)
}()

for {
select {
case <-l.ctx.Done():
Expand Down

0 comments on commit 2963ef2

Please sign in to comment.