internal: fix client send preface problems

[jeanbza]

This CL fixes three problems:

• In clientconn_state_transitions_test.go, each time we set up a server to send
  SETTINGS, we should also set up the server to read. This allows the client
  to successfully send its SETTINGS.

• In clientconn.go, we incorrectly transitioned into TRANSIENT FAILURE when
  creating an http2client returned an error. This should be handled in the outer
  resetTransport main reset loop. The reason this became a problem is that the
  outer resetTransport has very specific conditions around when to transition
  into TRANSIENT FAILURE that the egregious transition did not have. So, it
  could transition into TRANSIENT FAILURE after failing to dial, even if it
  was trying to connect to a non-final address in the list of addresses.

• In clientconn.go, we incorrectly stay in CONNECTING after createTransport when a server sends
  its connection preface but the client is not able to send its connection preface. This
  CL causes the addrconn to correctly enter TRANSIENT FAILURE when createTransport
  fails, even if a server preface was received. It does so by making
  ac.successfulHandshake to consider both server preface received as well as
  client preface sent.

[jeanbza]

Tested successfully on travis with -run TestStateTransitions -count 1000 -race -v: https://travis-ci.org/jadekler/grpc-go/builds/441973011?utm_medium=notification&utm_source=email

[dfawley]

In clientconn_state_transitions_test.go, each time we set up a server to send
SETTINGS, we should also set up the server to read. This allows the client
to successfully send its SETTINGS.

This condition may happen in practice, though. Should we have a test case for each server behavior?

[jeanbza]

In clientconn_state_transitions_test.go, each time we set up a server to send
SETTINGS, we should also set up the server to read. This allows the client
to successfully send its SETTINGS.

This condition may happen in practice, though. Should we have a test case for each server behavior?

I thought about that, but here's what that test looks like:

• Set up server to Accept and send preface receipt, but not read
• Expect either READY or TRANSIENT FAILURE [1]

Is that what you'd expect, given [1]? Or is there some behavior that's defined for this?

1: Apparently, you can do n, err := t.conn.Write(clientPreface) without a server having called Read and get a successful response. Some of the time, though, you get the error:

transport: failed to write client preface: write tcp 127.0.0.1:39066->127.0.0.1:38566: use of closed network connection

(despite the fact that lis.Close has not been called yet)

I naively would have expected a synchronous pipe on conn.Write until the server does conn.Read, or minimally an async-with-timeout API.

[jeanbza]

@dfawley PTAL. Added PipeListener, a new test to make sure client does not enter READY until both prefaces are sent/received, and code to do the needful.

Tests on travis with -run TestStateTransitions -count 1000 https://travis-ci.org/jadekler/grpc-go/builds/442905877.

[dfawley]

How about: lock, close p.c, unlock. In Dial: lock, check that p.c is open, make a new chan and push onto p.c, unlock? I think that avoids any potential races.

[dfawley]

LGTM.. I don't really like adding complexity to something that's already too complex, but I'm OK with this for now since we're aiming to change it soon.

[dfawley]

Just a couple minor things I noticed and one optional thing. Approving. Thanks!