spanner: canceled context is used to record stats

[nodirg]

I have been reading Spanner client code and stumbled on this:

google-cloud-go/spanner/session.go

Lines 1502 to 1517 in 626dcfc

	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
	err := ws.prepareForWrite(ctx)
	cancel()
	if err != nil {
	// Skip handling prepare error, session can be prepared in next
	// cycle.
	// Don't log about permission errors, which may be expected
	// (e.g. using read-only auth).
	serr := toSpannerError(err).(*Error)
	if serr.Code != codes.PermissionDenied {
	logf(hc.pool.sc.logger, "Failed to prepare session, error: %v", serr)
	}
	}
	hc.pool.recycle(ws)
	hc.pool.mu.Lock()
	hc.pool.decNumBeingPreparedLocked(ctx)

note here that ctx is canceled in L1504 and then that context is used in L1517.

I don't have concrete problem that this caused, but this code does not look correct. Perhaps ctx variable in L1502 should be renamed

[skuruppu]

@olavloite, if you could also take a look at this, that would be great.

CC @hengfengli

[olavloite]

@nodirg Thank you for your report. In this specific case it is as far as I can tell not really a problem, but I do agree with you that it looks a little bit suspicious. The reason that it is not really a problem in this case, is that the context is only used to include the tags that should be included with the statistics that are recorded.
I've moved the cancel() call to right after where it is used for the stats to prevent any confusion.