send messages with select to avoid deadlocks

Fixes #287.

.github/ISSUE_TEMPLATE

@@ -0,0 +1,15 @@
+#### What versions are you running?
+
+<pre>
+$ go list -m github.com/chromedp/chromedp
+$ chromium --version
+$ go version
+</pre>
+
+#### What did you do?
+
+
+#### What did you expect to see?
+
+
+#### What did you see instead?

.travis.yml

@@ -1,7 +1,7 @@
 language: go
 go:
   - 1.10.x
-  - tip
+  - 1.11.x
 addons:
   apt:
     chrome: stable

chromedp.go

@@ -239,6 +239,7 @@ func (c *CDP) SetHandlerByID(id string) error {
 
 	if i, ok := c.handlerMap[id]; ok {
 		c.cur = c.handlers[i]
+		return nil
 	}
 
 	return fmt.Errorf("no handler associated with target id %s", id)

chromedp_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"log"
 	"os"
-	"os/exec"
 	"path"
 	"testing"
 	"time"
@@ -78,16 +77,9 @@ func TestMain(m *testing.M) {
 
 	// its worth noting that newer versions of chrome (64+) run much faster
 	// than older ones -- same for headless_shell ...
-	execPath := runner.DefaultChromePath
+	execPath := os.Getenv("CHROMEDP_TEST_RUNNER")
-	if testRunner := os.Getenv("CHROMEDP_TEST_RUNNER"); testRunner != "" {
+	if execPath == "" {
-		execPath = testRunner
+		execPath = runner.LookChromeNames("headless_shell")
-	} else {
-		// use headless_shell, if on path
-		var hsPath string
-		hsPath, err = exec.LookPath("headless_shell")
-		if err == nil {
-			execPath = hsPath
-		}
 	}
 	cliOpts = append(cliOpts, runner.ExecPath(execPath))
 

go.mod

@@ -1,10 +1,9 @@
 module github.com/chromedp/chromedp
 
 require (
-	github.com/chromedp/cdproto v0.0.0-20180713053126-e314dc107013
+	github.com/chromedp/cdproto v0.0.0-20190327003620-8d5e1d04ce19
-	github.com/disintegration/imaging v1.4.2
+	github.com/disintegration/imaging v1.6.0
-	github.com/gorilla/websocket v1.2.0
+	github.com/gorilla/websocket v1.4.0
-	github.com/knq/sysutil v0.0.0-20180306023629-0218e141a794
+	github.com/mailru/easyjson v0.0.0-20190312143242-1de009706dbe
-	github.com/mailru/easyjson v0.0.0-20180606163543-3fdea8d05856
+	golang.org/x/image v0.0.0-20190220214146-31aff87c08e9 // indirect
-	golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81
 )

go.sum

@@ -1,19 +1,16 @@
-github.com/chromedp/cdproto v0.0.0-20180522032958-55db67b53f25/go.mod h1:C2GPAraqdt1KfZU7aSmx1XUgarNq/3JmxevQkmCjOVs=
+github.com/chromedp/cdproto v0.0.0-20190327003620-8d5e1d04ce19 h1:KOdZXVcB8L3zR4ZsMAnviYJFIgfRP/iYSEzXl7rYXhc=
-github.com/chromedp/cdproto v0.0.0-20180703215205-c125a34ea3b3 h1:4b4LwyHW4sf6zXZqWsKMYoICFLWGhaqwpEHlnQMN5SE=
+github.com/chromedp/cdproto v0.0.0-20190327003620-8d5e1d04ce19/go.mod h1:xquOK9dIGFlLaIGI4c6IyfLI/Gz0LiYYuJtzhsUODgI=
-github.com/chromedp/cdproto v0.0.0-20180703215205-c125a34ea3b3/go.mod h1:C2GPAraqdt1KfZU7aSmx1XUgarNq/3JmxevQkmCjOVs=
+github.com/disintegration/imaging v1.6.0 h1:nVPXRUUQ36Z7MNf0O77UzgnOb1mkMMor7lmJMJXc/mA=
-github.com/chromedp/cdproto v0.0.0-20180713053126-e314dc107013 h1:8nmuTwCseJcww39MvVHI59223+PxSzn6g3cl8ChF0/4=
+github.com/disintegration/imaging v1.6.0/go.mod h1:xuIt+sRxDFrHS0drzXUlCJthkJ8k7lkkUojDSR247MQ=
-github.com/chromedp/cdproto v0.0.0-20180713053126-e314dc107013/go.mod h1:C2GPAraqdt1KfZU7aSmx1XUgarNq/3JmxevQkmCjOVs=
+github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
-github.com/disintegration/imaging v1.4.2 h1:BSVxoYQ2NfLdvIGCDD8GHgBV5K0FCEsc0d/6FxQII3I=
+github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
-github.com/disintegration/imaging v1.4.2/go.mod h1:9B/deIUIrliYkyMTuXJd6OUFLcrZ2tf+3Qlwnaf/CjU=
+github.com/knq/sysutil v0.0.0-20181215143952-f05b59f0f307 h1:vl4eIlySbjertFaNwiMjXsGrFVK25aOWLq7n+3gh2ls=
-github.com/gorilla/websocket v1.2.0 h1:VJtLvh6VQym50czpZzx07z/kw9EgAxI3x1ZB8taTMQQ=
+github.com/knq/sysutil v0.0.0-20181215143952-f05b59f0f307/go.mod h1:BjPj+aVjl9FW/cCGiF3nGh5v+9Gd3VCgBQbod/GlMaQ=
-github.com/gorilla/websocket v1.2.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
+github.com/mailru/easyjson v0.0.0-20180823135443-60711f1a8329/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
-github.com/knq/sysutil v0.0.0-20180306023629-0218e141a794 h1:hgWKTlyruPI7k8W+0FmTMLf+8d2KPxyzTxsfDDQhNp8=
+github.com/mailru/easyjson v0.0.0-20190312143242-1de009706dbe h1:W/GaMY0y69G4cFlmsC6B9sbuo2fP8OFP1ABjt4kPz+w=
-github.com/knq/sysutil v0.0.0-20180306023629-0218e141a794/go.mod h1:BjPj+aVjl9FW/cCGiF3nGh5v+9Gd3VCgBQbod/GlMaQ=
+github.com/mailru/easyjson v0.0.0-20190312143242-1de009706dbe/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
-github.com/mailru/easyjson v0.0.0-20180323154445-8b799c424f57/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
-github.com/mailru/easyjson v0.0.0-20180606163543-3fdea8d05856 h1:hOnidOuIWNsFRPcxxStGeN3NNm4n4+w6KJ9cVJIh70o=
-github.com/mailru/easyjson v0.0.0-20180606163543-3fdea8d05856/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
-golang.org/x/image v0.0.0-20180403161127-f315e4403028/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
-golang.org/x/image v0.0.0-20180628062038-cc896f830ced h1:2QsAEqOy4Mp+V4HL2Wr1iBNpZWaL72EvTO4oj5bmr5w=
-golang.org/x/image v0.0.0-20180628062038-cc896f830ced/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
 golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81 h1:00VmoueYNlNz/aHIilyyQz/MHSqGoWJzpFv/HW8xpzI=
 golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
+golang.org/x/image v0.0.0-20190220214146-31aff87c08e9 h1:+vH8qNweCrORN49012OX3h0oWEXO3p+rRnpAGQinddk=
+golang.org/x/image v0.0.0-20190220214146-31aff87c08e9/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

handler.go

@@ -89,7 +89,7 @@ func (h *TargetHandler) Run(ctxt context.Context) error {
 	h.qres = make(chan *cdproto.Message)
 	h.qevents = make(chan *cdproto.Message)
 	h.res = make(map[int64]chan *cdproto.Message)
-	h.detached = make(chan *inspector.EventDetached)
+	h.detached = make(chan *inspector.EventDetached, 1)
 	h.pageWaitGroup = new(sync.WaitGroup)
 	h.domWaitGroup = new(sync.WaitGroup)
 	h.Unlock()
@@ -155,10 +155,18 @@ func (h *TargetHandler) run(ctxt context.Context) {
 
 				switch {
 				case msg.Method != "":
-					h.qevents <- msg
+					select {
+					case h.qevents <- msg:
+					case <-ctxt.Done():
+						return
+					}
 
 				case msg.ID != 0:
-					h.qres <- msg
+					select {
+					case h.qres <- msg:
+					case <-ctxt.Done():
+						return
+					}
 
 				default:
 					h.errf("ignoring malformed incoming message (missing id or method): %#v", msg)
@@ -226,6 +234,15 @@ func (h *TargetHandler) processEvent(ctxt context.Context, msg *cdproto.Message)
 	if msg == nil {
 		return ErrChannelClosed
 	}
+	switch msg.Method {
+	case "Page.frameClearedScheduledNavigation",
+		"Page.frameScheduledNavigation":
+		// These events are now deprecated, and UnmarshalMessage panics
+		// when they are received from Chrome. For now, to avoid panics
+		// and compile errors, and to fix chromedp v0 when installed via
+		// 'go get -u', skip the events here.
+		return nil
+	}
 
 	// unmarshal
 	ev, err := cdproto.UnmarshalMessage(msg)
@@ -346,10 +363,14 @@ func (h *TargetHandler) Execute(ctxt context.Context, methodType string, params
 	h.resrw.Unlock()
 
 	// queue message
-	h.qcmd <- &cdproto.Message{
+	select {
+	case h.qcmd <- &cdproto.Message{
 		ID:     id,
 		Method: cdproto.MethodType(methodType),
 		Params: paramsBuf,
+	}:
+	case <- ctxt.Done():
+		return ctxt.Err()
 	}
 
 	errch := make(chan error, 1)
@@ -530,13 +551,9 @@ func (h *TargetHandler) pageEvent(ctxt context.Context, ev interface{}) {
 	case *page.EventFrameStoppedLoading:
 		id, op = e.FrameID, frameStoppedLoading
 
-	case *page.EventFrameScheduledNavigation:
-		id, op = e.FrameID, frameScheduledNavigation
-
-	case *page.EventFrameClearedScheduledNavigation:
-		id, op = e.FrameID, frameClearedScheduledNavigation
-
 		// ignored events
+	case *page.EventFrameRequestedNavigation:
+		return
 	case *page.EventDomContentEventFired:
 		return
 	case *page.EventLoadEventFired:

input_test.go

@@ -21,10 +21,10 @@ const (
 )
 
 func TestMouseClickXY(t *testing.T) {
-	var err error
-
 	t.Parallel()
 
+	var err error
+
 	c := testAllocate(t, "input.html")
 	defer c.Release()
 
@@ -78,6 +78,8 @@ func TestMouseClickXY(t *testing.T) {
 }
 
 func TestMouseClickNode(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel, exp string
 		opt      MouseOption
@@ -128,6 +130,8 @@ func TestMouseClickNode(t *testing.T) {
 }
 
 func TestMouseClickOffscreenNode(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		exp int
@@ -186,6 +190,8 @@ func TestMouseClickOffscreenNode(t *testing.T) {
 }
 
 func TestKeyAction(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel, exp string
 		by       QueryOption
@@ -238,6 +244,8 @@ func TestKeyAction(t *testing.T) {
 }
 
 func TestKeyActionNode(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel, exp string
 		by       QueryOption

pool.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"net"
 	"sync"
 
 	"github.com/chromedp/chromedp/runner"
@@ -70,6 +71,18 @@ func (p *Pool) Allocate(ctxt context.Context, opts ...runner.CommandLineOption)
 
 	r := p.next(ctxt)
 
+	// Check if the port is available first. If it's not, Chrome will print
+	// an "address already in use" error, but it will otherwise keep
+	// running. This can lead to Allocate succeeding, while the chrome
+	// process isn't actually listening on the port we need.
+	l, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", r.port))
+	if err != nil {
+		// we can't use this port, e.g. address already in use
+		p.errf("pool could not allocate runner on port %d: %v", r.port, err)
+		return nil, err
+	}
+	l.Close()
+
 	p.debugf("pool allocating %d", r.port)
 
 	// create runner

pool_test.go

@@ -0,0 +1,47 @@
+package chromedp
+
+import (
+	"context"
+	"net"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+func TestAllocatePortInUse(t *testing.T) {
+	t.Parallel()
+
+	// take a random available port
+	l, err := net.Listen("tcp4", "localhost:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	ctxt, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// make the pool use the port already in use via a port range
+	_, portStr, _ := net.SplitHostPort(l.Addr().String())
+	port, _ := strconv.Atoi(portStr)
+	pool, err := NewPool(
+		PortRange(port, port+1),
+		// skip the error log from the used port
+		PoolLog(nil, nil, func(string, ...interface{}) {}),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	c, err := pool.Allocate(ctxt)
+	if err != nil {
+		want := "address already in use"
+		got := err.Error()
+		if !strings.Contains(got, want) {
+			t.Fatalf("wanted error to contain %q, but got %q", want, got)
+		}
+	} else {
+		t.Fatal("wanted Allocate to error if port is in use")
+		c.Release()
+	}
+}

query_test.go

@@ -190,6 +190,8 @@ func TestText(t *testing.T) {
 }
 
 func TestClear(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -244,6 +246,8 @@ func TestClear(t *testing.T) {
 }
 
 func TestReset(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel   string
 		by    QueryOption
@@ -315,6 +319,8 @@ func TestValue(t *testing.T) {
 }
 
 func TestSetValue(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -442,6 +448,8 @@ func TestAttributesAll(t *testing.T) {
 }
 
 func TestSetAttributes(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel   string
 		by    QueryOption
@@ -547,6 +555,8 @@ func TestAttributeValue(t *testing.T) {
 }
 
 func TestSetAttributeValue(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel  string
 		by   QueryOption
@@ -589,6 +599,8 @@ func TestSetAttributeValue(t *testing.T) {
 }
 
 func TestRemoveAttribute(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel  string
 		by   QueryOption
@@ -626,6 +638,8 @@ func TestRemoveAttribute(t *testing.T) {
 }
 
 func TestClick(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -666,6 +680,8 @@ func TestClick(t *testing.T) {
 }
 
 func TestDoubleClick(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -703,6 +719,8 @@ func TestDoubleClick(t *testing.T) {
 }
 
 func TestSendKeys(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel  string
 		by   QueryOption
@@ -773,6 +791,8 @@ func TestScreenshot(t *testing.T) {
 }
 
 func TestSubmit(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -813,6 +833,8 @@ func TestSubmit(t *testing.T) {
 }
 
 func TestComputedStyle(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -870,6 +892,8 @@ func TestComputedStyle(t *testing.T) {
 }
 
 func TestMatchedStyle(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		sel string
 		by  QueryOption
@@ -940,9 +964,6 @@ func TestFileUpload(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	c := testAllocate(t, "")
-	defer c.Release()
-
 	tests := []struct {
 		a Action
 	}{
@@ -952,6 +973,10 @@ func TestFileUpload(t *testing.T) {
 
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) {
+			// TODO: refactor the test so the subtests can run in
+			// parallel
+			//t.Parallel()
+
 			c := testAllocate(t, "")
 			defer c.Release()
 

runner/runner.go

@@ -213,7 +213,8 @@ func (r *Runner) Start(ctxt context.Context, opts ...string) error {
 	return nil
 }
 
-// Shutdown shuts down the Chrome process.
+// Shutdown shuts down the Chrome process.  Currently only has support for
+// SIGTERM in darwin and linux systems
 func (r *Runner) Shutdown(ctxt context.Context, opts ...client.Option) error {
 	var err error
 
@@ -241,12 +242,15 @@ func (r *Runner) Shutdown(ctxt context.Context, opts ...client.Option) error {
 		}
 	}
 
-	// osx applications do not automatically exit when all windows (ie, tabs)
+	// osx and linux applications do not automatically exit when all windows (ie, tabs)
 	// closed, so send SIGTERM.
 	//
 	// TODO: add other behavior here for more process options on shutdown?
-	if runtime.GOOS == "darwin" && r.cmd != nil && r.cmd.Process != nil {
+	if r.cmd != nil && r.cmd.Process != nil {
-		return r.cmd.Process.Signal(syscall.SIGTERM)
+		switch runtime.GOOS {
+		case "darwin", "linux":
+			return r.cmd.Process.Signal(syscall.SIGTERM)
+		}
 	}
 
 	return nil

runner/runner_windows.go

@@ -24,21 +24,3 @@ func KillProcessGroup(m map[string]interface{}) error {
 func ForceKill(m map[string]interface{}) error {
 	return nil
 }
-
-// EdgeDiagnosticsAdapterWithPath is a command line option to specify using the
-// Microsoft Edge Diagnostics adapter at the specified path.
-func EdgeDiagnosticsAdapterWithPathAndPort(path string, port int) CommandLineOption {
-	return func(m map[string]interface{}) error {
-		m["exec-path"] = path
-		m["port"] = port
-		return nil
-	}
-}
-
-// EdgeDiagnosticsAdapter is a command line option to specify using the
-// Microsoft Edge Diagnostics adapter found on the path.
-//
-// If the
-func EdgeDiagnosticsAdapter() CommandLineOption {
-	return EdgeDiagnosticsAdapterWithPathAndPort(findEdgePath(), 9222)
-}

sel.go

@@ -80,7 +80,7 @@ func (s *Selector) Do(ctxt context.Context, h cdp.Executor) error {
 // are invalidated prior to finishing the selector's by, wait, check, and after
 // funcs.
 func (s *Selector) run(ctxt context.Context, h *TargetHandler) chan error {
-	ch := make(chan error)
+	ch := make(chan error, 1)
 
 	go func() {
 		defer close(ch)