FAQ
Reviewers: golang-dev1,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go.crypto


Description:
go.crypto/ssh: fix race on mock ssh network connection

Fixes issue 5138.

Please review this at https://codereview.appspot.com/9929043/

Affected files:
    M ssh/test/test_unix_test.go


Index: ssh/test/test_unix_test.go
===================================================================
--- a/ssh/test/test_unix_test.go
+++ b/ssh/test/test_unix_test.go
@@ -24,6 +24,7 @@
    "os/exec"
    "os/user"
    "path/filepath"
+ "sync"
    "testing"
    "text/template"
    "time"
@@ -112,13 +113,12 @@
     s.t.Fatal(err)
    }
    s.cmd.Stderr = os.Stderr // &s.output
- err = s.cmd.Start()
- if err != nil {
+ if err := s.cmd.Start(); err != nil {
     s.t.Fail()
     s.Shutdown()
     s.t.Fatalf("s.cmd.Start: %v", err)
    }
- conn, err := ssh.Client(&client{stdin, stdout}, config)
+ conn, err := ssh.Client(&client{wc: stdin, r: stdout}, config)
    if err != nil {
     s.t.Fail()
     s.Shutdown()
@@ -136,25 +136,77 @@
    }
    if s.t.Failed() {
     // log any output from sshd process
- s.t.Log(s.output.String())
+ s.t.Logf("sshd: %q", s.output.String())
    }
    s.cleanup()
   }

   // client wraps a pair of Reader/WriteClosers to implement the
-// net.Conn interface.
+// net.Conn interface. Importantly, client also mocks the
+// ability of net.Conn to support concurrent calls to Read/Write
+// and close. See golang.org/issue/5138 for more details.
   type client struct {
- io.WriteCloser
- io.Reader
+ wc io.WriteCloser
+ r io.Reader
+ sync.Mutex // protecets sysref and closing
+ sysref int
+ closing bool
   }

+var errClosing = errors.New("use of closed network connection")
+
   func (c *client) LocalAddr() net.Addr { return nil }
   func (c *client) RemoteAddr() net.Addr { return nil }
   func (c *client) SetDeadline(time.Time) error { return nil }
   func (c *client) SetReadDeadline(time.Time) error { return nil }
   func (c *client) SetWriteDeadline(time.Time) error { return nil }

-// newServer returns a new mock ssh server.
+func (c *client) incref(closing bool) error {
+ c.Lock()
+ defer c.Unlock()
+ if c.closing {
+ return errClosing
+ }
+ c.sysref++
+ if closing {
+ c.closing = true
+ }
+ return nil
+}
+
+func (c *client) decref() {
+ c.Lock()
+ defer c.Unlock()
+ c.sysref--
+ if c.closing && c.sysref == 0 {
+ c.wc.Close()
+ }
+}
+
+func (c *client) Close() error {
+ if err := c.incref(true); err != nil {
+ return err
+ }
+ c.decref()
+ return nil
+}
+
+func (c *client) Read(b []byte) (int, error) {
+ if err := c.incref(false); err != nil {
+ return 0, err
+ }
+ defer c.decref()
+ return c.r.Read(b)
+}
+
+func (c *client) Write(b []byte) (int, error) {
+ if err := c.incref(false); err != nil {
+ return 0, err
+ }
+ defer c.decref()
+ return c.wc.Write(b)
+}
+
   func newServer(t *testing.T) *server {
    dir, err := ioutil.TempDir("", "sshtest")
    if err != nil {


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Dave at Jun 1, 2013 at 1:25 am
    Hello golang-dev@googlegroups.com, fullung@gmail.com (cc:
    golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/9929043/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Fullung at Jun 1, 2013 at 6:56 am
    Looks pretty good. I still need to take a closer look at the mock conn.

    https://codereview.appspot.com/download/issue8402046_4001.diff also had
    a:

    s.cmd.Wait()

    I think that's still a good idea.

    Fixes issue 4703 too?

    https://codereview.appspot.com/9929043/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dave Cheney at Jun 1, 2013 at 6:59 am
    s.cmd.Wait()
    That was incorporated into s.Shutdown by dsymonds a while back. Please
    let me know if I am missing something.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJun 1, '13 at 12:54a
activeJun 1, '13 at 6:59a
posts4
users2
websitegolang.org

2 users in discussion

Dave Cheney: 3 posts Fullung: 1 post

People

Translate

site design / logo © 2021 Grokbase