Skip to content

Commit 5935531

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
internal/jsonrpc2_v2: remove “Box” suffix from channel field names
With the suffixes I end up a little lost in the “box” noise while I'm reading — the channel ops alone suffice to make the storage mechanism clear. (To me, the mechanism of storing a value in a 1-buffered channel is conceptually similar to storing it in a pointer, atomic.Pointer, or similar — and we don't generally name those with a suffix either.) For golang/go#46047. For golang/go#46520. For golang/go#49387. Change-Id: I7f58a9ac532f597fe49ed70606d89bd8cbe33b55 Reviewed-on: https://go-review.googlesource.com/c/tools/+/443355 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Bryan Mills <[email protected]> Auto-Submit: Bryan Mills <[email protected]> Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent fd32990 commit 5935531

File tree

2 files changed

+48
-48
lines changed

2 files changed

+48
-48
lines changed

internal/jsonrpc2_v2/conn.go

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,17 @@ type Connection struct {
5151
closeOnce sync.Once
5252
closer io.Closer
5353

54-
writerBox chan Writer
55-
outgoingBox chan map[ID]chan<- *Response
56-
incomingBox chan map[ID]*incoming
57-
async *async
54+
writer chan Writer
55+
outgoing chan map[ID]chan<- *Response
56+
incoming chan map[ID]*incoming
57+
async *async
5858
}
5959

6060
type AsyncCall struct {
61-
id ID
62-
response chan *Response // the channel a response will be delivered on
63-
resultBox chan asyncResult
64-
endSpan func() // close the tracing span when all processing for the message is complete
61+
id ID
62+
response chan *Response // the channel a response will be delivered on
63+
result chan asyncResult
64+
endSpan func() // close the tracing span when all processing for the message is complete
6565
}
6666

6767
type asyncResult struct {
@@ -87,11 +87,11 @@ func (o ConnectionOptions) Bind(context.Context, *Connection) (ConnectionOptions
8787
// This is used by the Dial and Serve functions to build the actual connection.
8888
func newConnection(ctx context.Context, rwc io.ReadWriteCloser, binder Binder) (*Connection, error) {
8989
c := &Connection{
90-
closer: rwc,
91-
writerBox: make(chan Writer, 1),
92-
outgoingBox: make(chan map[ID]chan<- *Response, 1),
93-
incomingBox: make(chan map[ID]*incoming, 1),
94-
async: newAsync(),
90+
closer: rwc,
91+
writer: make(chan Writer, 1),
92+
outgoing: make(chan map[ID]chan<- *Response, 1),
93+
incoming: make(chan map[ID]*incoming, 1),
94+
async: newAsync(),
9595
}
9696

9797
options, err := binder.Bind(ctx, c)
@@ -107,8 +107,8 @@ func newConnection(ctx context.Context, rwc io.ReadWriteCloser, binder Binder) (
107107
if options.Handler == nil {
108108
options.Handler = defaultHandler{}
109109
}
110-
c.outgoingBox <- make(map[ID]chan<- *Response)
111-
c.incomingBox <- make(map[ID]*incoming)
110+
c.outgoing <- make(map[ID]chan<- *Response)
111+
c.incoming <- make(map[ID]*incoming)
112112
// the goroutines started here will continue until the underlying stream is closed
113113
reader := options.Framer.Reader(rwc)
114114
readToQueue := make(chan *incoming)
@@ -119,7 +119,7 @@ func newConnection(ctx context.Context, rwc io.ReadWriteCloser, binder Binder) (
119119

120120
// releaseing the writer must be the last thing we do in case any requests
121121
// are blocked waiting for the connection to be ready
122-
c.writerBox <- options.Framer.Writer(rwc)
122+
c.writer <- options.Framer.Writer(rwc)
123123
return c, nil
124124
}
125125

@@ -154,14 +154,14 @@ func (c *Connection) Notify(ctx context.Context, method string, params interface
154154
// If sending the call failed, the response will be ready and have the error in it.
155155
func (c *Connection) Call(ctx context.Context, method string, params interface{}) *AsyncCall {
156156
result := &AsyncCall{
157-
id: Int64ID(atomic.AddInt64(&c.seq, 1)),
158-
resultBox: make(chan asyncResult, 1),
157+
id: Int64ID(atomic.AddInt64(&c.seq, 1)),
158+
result: make(chan asyncResult, 1),
159159
}
160160
// generate a new request identifier
161161
call, err := NewCall(result.id, method, params)
162162
if err != nil {
163163
//set the result to failed
164-
result.resultBox <- asyncResult{err: fmt.Errorf("marshaling call parameters: %w", err)}
164+
result.result <- asyncResult{err: fmt.Errorf("marshaling call parameters: %w", err)}
165165
return result
166166
}
167167
ctx, endSpan := event.Start(ctx, method,
@@ -175,7 +175,7 @@ func (c *Connection) Call(ctx context.Context, method string, params interface{}
175175
// are racing the response.
176176
// rchan is buffered in case the response arrives without a listener.
177177
result.response = make(chan *Response, 1)
178-
outgoing, ok := <-c.outgoingBox
178+
outgoing, ok := <-c.outgoing
179179
if !ok {
180180
// If the call failed due to (say) an I/O error or broken pipe, attribute it
181181
// as such. (If the error is nil, then the connection must have been shut
@@ -193,7 +193,7 @@ func (c *Connection) Call(ctx context.Context, method string, params interface{}
193193
return result
194194
}
195195
outgoing[result.id] = result.response
196-
c.outgoingBox <- outgoing
196+
c.outgoing <- outgoing
197197
// now we are ready to send
198198
if err := c.write(ctx, call); err != nil {
199199
// sending failed, we will never get a response, so deliver a fake one
@@ -212,8 +212,8 @@ func (a *AsyncCall) ID() ID { return a.id }
212212
// returned, or a call that failed to send in the first place.
213213
func (a *AsyncCall) IsReady() bool {
214214
select {
215-
case r := <-a.resultBox:
216-
a.resultBox <- r
215+
case r := <-a.result:
216+
a.result <- r
217217
return true
218218
default:
219219
return false
@@ -236,14 +236,14 @@ func (a *AsyncCall) Await(ctx context.Context, result interface{}) error {
236236
r.result = response.Result
237237
event.Label(ctx, tag.StatusCode.Of("OK"))
238238
}
239-
case r = <-a.resultBox:
239+
case r = <-a.result:
240240
// result already available
241241
case <-ctx.Done():
242242
event.Label(ctx, tag.StatusCode.Of("CANCELLED"))
243243
return ctx.Err()
244244
}
245245
// refill the box for the next caller
246-
a.resultBox <- r
246+
a.result <- r
247247
// and unpack the result
248248
if r.err != nil {
249249
return r.err
@@ -259,8 +259,8 @@ func (a *AsyncCall) Await(ctx context.Context, result interface{}) error {
259259
// Respond must be called exactly once for any message for which a handler
260260
// returns ErrAsyncResponse. It must not be called for any other message.
261261
func (c *Connection) Respond(id ID, result interface{}, rerr error) error {
262-
pending := <-c.incomingBox
263-
defer func() { c.incomingBox <- pending }()
262+
pending := <-c.incoming
263+
defer func() { c.incoming <- pending }()
264264
entry, found := pending[id]
265265
if !found {
266266
return nil
@@ -277,8 +277,8 @@ func (c *Connection) Respond(id ID, result interface{}, rerr error) error {
277277
// not cause any messages that have not arrived yet with that ID to be
278278
// cancelled.
279279
func (c *Connection) Cancel(id ID) {
280-
pending := <-c.incomingBox
281-
defer func() { c.incomingBox <- pending }()
280+
pending := <-c.incoming
281+
defer func() { c.incoming <- pending }()
282282
if entry, found := pending[id]; found && entry.cancel != nil {
283283
entry.cancel()
284284
entry.cancel = nil
@@ -310,10 +310,10 @@ func (c *Connection) readIncoming(ctx context.Context, reader Reader, toQueue ch
310310
defer func() {
311311
// Retire any outgoing requests that were still in flight.
312312
// With the Reader no longer being processed, they necessarily cannot receive a response.
313-
outgoing := <-c.outgoingBox
314-
close(c.outgoingBox) // Prevent new outgoing requests, which would deadlock.
315-
for id, responseBox := range outgoing {
316-
responseBox <- &Response{ID: id, Error: err}
313+
outgoing := <-c.outgoing
314+
close(c.outgoing) // Prevent new outgoing requests, which would deadlock.
315+
for id, response := range outgoing {
316+
response <- &Response{ID: id, Error: err}
317317
}
318318

319319
close(toQueue)
@@ -352,9 +352,9 @@ func (c *Connection) readIncoming(ctx context.Context, reader Reader, toQueue ch
352352
// if the request is a call, add it to the incoming map so it can be
353353
// cancelled by id
354354
if msg.IsCall() {
355-
pending := <-c.incomingBox
355+
pending := <-c.incoming
356356
pending[msg.ID] = entry
357-
c.incomingBox <- pending
357+
c.incoming <- pending
358358
}
359359
// send the message to the incoming queue
360360
toQueue <- entry
@@ -368,10 +368,10 @@ func (c *Connection) readIncoming(ctx context.Context, reader Reader, toQueue ch
368368

369369
func (c *Connection) incomingResponse(msg *Response) {
370370
var response chan<- *Response
371-
if outgoing, ok := <-c.outgoingBox; ok {
371+
if outgoing, ok := <-c.outgoing; ok {
372372
response = outgoing[msg.ID]
373373
delete(outgoing, msg.ID)
374-
c.outgoingBox <- outgoing
374+
c.outgoing <- outgoing
375375
}
376376
if response != nil {
377377
response <- msg
@@ -468,8 +468,8 @@ func (c *Connection) deliverMessages(ctx context.Context, handler Handler, fromQ
468468
func (c *Connection) reply(entry *incoming, result interface{}, rerr error) {
469469
if entry.request.IsCall() {
470470
// we have a call finishing, remove it from the incoming map
471-
pending := <-c.incomingBox
472-
defer func() { c.incomingBox <- pending }()
471+
pending := <-c.incoming
472+
defer func() { c.incoming <- pending }()
473473
delete(pending, entry.request.ID)
474474
}
475475
if err := c.respond(entry, result, rerr); err != nil {
@@ -527,8 +527,8 @@ func (c *Connection) respond(entry *incoming, result interface{}, rerr error) er
527527
// write is used by all things that write outgoing messages, including replies.
528528
// it makes sure that writes are atomic
529529
func (c *Connection) write(ctx context.Context, msg Message) error {
530-
writer := <-c.writerBox
531-
defer func() { c.writerBox <- writer }()
530+
writer := <-c.writer
531+
defer func() { c.writer <- writer }()
532532
n, err := writer.Write(ctx, msg)
533533
event.Metric(ctx, tag.SentBytes.Of(n))
534534
return err

internal/jsonrpc2_v2/jsonrpc2_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ type binder struct {
7777
type handler struct {
7878
conn *jsonrpc2.Connection
7979
accumulator int
80-
waitersBox chan map[string]chan struct{}
80+
waiters chan map[string]chan struct{}
8181
calls map[string]*jsonrpc2.AsyncCall
8282
}
8383

@@ -256,11 +256,11 @@ func verifyResults(t *testing.T, method string, results interface{}, expect inte
256256

257257
func (b binder) Bind(ctx context.Context, conn *jsonrpc2.Connection) (jsonrpc2.ConnectionOptions, error) {
258258
h := &handler{
259-
conn: conn,
260-
waitersBox: make(chan map[string]chan struct{}, 1),
261-
calls: make(map[string]*jsonrpc2.AsyncCall),
259+
conn: conn,
260+
waiters: make(chan map[string]chan struct{}, 1),
261+
calls: make(map[string]*jsonrpc2.AsyncCall),
262262
}
263-
h.waitersBox <- make(map[string]chan struct{})
263+
h.waiters <- make(map[string]chan struct{})
264264
if b.runTest != nil {
265265
go b.runTest(h)
266266
}
@@ -272,8 +272,8 @@ func (b binder) Bind(ctx context.Context, conn *jsonrpc2.Connection) (jsonrpc2.C
272272
}
273273

274274
func (h *handler) waiter(name string) chan struct{} {
275-
waiters := <-h.waitersBox
276-
defer func() { h.waitersBox <- waiters }()
275+
waiters := <-h.waiters
276+
defer func() { h.waiters <- waiters }()
277277
waiter, found := waiters[name]
278278
if !found {
279279
waiter = make(chan struct{})

0 commit comments

Comments
 (0)