mcp: address some comments from #234 (#248)

I inadvertently merged #234 without pushing my latest patches. This CL
adds the missing changes.

Author: findleyr

mcp/cmd_test.go

@@ -129,7 +129,9 @@ func TestServerInterrupt(t *testing.T) {
 	}()
 
 	// send a signal to the server process to terminate it
-	cmd.Process.Signal(os.Interrupt)
+	if err := cmd.Process.Signal(os.Interrupt); err != nil {
+		t.Fatal(err)
+	}
 
 	// wait for the server to exit
 	// TODO: use synctest when available
@@ -162,6 +164,11 @@ func TestStdioContextCancellation(t *testing.T) {
 	}
 
 	// Sleep to make it more likely that the server is blocked in the read loop.
+	//
+	// This sleep isn't necessary for the test to pass, but *was* necessary for
+	// it to fail, before closing was fixed. Unfortunately, it is too invasive a
+	// change to have the jsonrpc2 package signal across packages when it is
+	// actually blocked in its read loop.
 	time.Sleep(100 * time.Millisecond)
 
 	onExit := make(chan struct{})
@@ -170,7 +177,9 @@ func TestStdioContextCancellation(t *testing.T) {
 		close(onExit)
 	}()
 
-	cmd.Process.Signal(os.Interrupt)
+	if err := cmd.Process.Signal(os.Interrupt); err != nil {
+		t.Fatal(err)
+	}
 
 	select {
 	case <-time.After(5 * time.Second):

mcp/transport.go

@@ -318,9 +318,9 @@ func newIOConn(rwc io.ReadWriteCloser) *ioConn {
 	// Start a goroutine for reads, so that we can select on the incoming channel
 	// in [ioConn.Read] and unblock the read as soon as Close is called (see #224).
 	//
-	// This leaks a goroutine, but that is unavoidable since AFAIK there is no
-	// (easy and portable) way to guarantee that reads of stdin are unblocked
-	// when closed.
+	// This leaks a goroutine if rwc.Read does not unblock after it is closed,
+	// but that is unavoidable since AFAIK there is no (easy and portable) way to
+	// guarantee that reads of stdin are unblocked when closed.
 	go func() {
 		dec := json.NewDecoder(rwc)
 		for {