tree 014b97ee08793bd7a14939c1aa36e65a83710605
parent 76135f99d5a036a067de493ecd00a85bd4c567bc
author Michael Stapelberg <stapelberg@golang.org> 1730465407 +0100
committer Michael Stapelberg <stapelberg@golang.org> 1730710749 +0000

internal/impl: fix TestMarshalMessageSetLazyRace (was a no-op!)

This test was accidentally migrated from proto.GetExtension to
proto.HasExtension in a semi-automated change and nobody noticed.

proto.HasExtension does not actually trigger lazy extension decoding.
I verified this using the dlv debugger:

% go test -tags protolegacy -c -gcflags "all=-N -l"
% dlv exec ./impl.test
Type 'help' for list of commands.
(dlv) r -test.run=TestMarshalMessageSetLazyRace -test.v
Process restarted with PID 3586699
(dlv) b TestMarshalMessageSetLazyRace
Breakpoint 1 set at 0xac9bf6 for google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500
(dlv) c
=== RUN   TestMarshalMessageSetLazyRace
> [Breakpoint 1] google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (hits goroutine(32):1 total:1) (PC: 0xac9bf6)
=> 500:	func TestMarshalMessageSetLazyRace(t *testing.T) {
(dlv) b ExtensionField.lazyInit
Breakpoint 2 set at 0x8da076 for google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123
(dlv) c
--- PASS: TestMarshalMessageSetLazyRace (4.07s)
PASS
Process 3586699 has exited with status 0
(dlv) exit

Note how breakpoint 2 is unexpectedly not hit!

Here is how the output changes with my fix:

% go test -tags protolegacy -c -gcflags "all=-N -l"
% dlv --check-go-version=false exec ./impl.test
Type 'help' for list of commands.
(dlv) r -test.run=TestMarshalMessageSetLazyRace -test.v
Process restarted with PID 3587344
(dlv) b TestMarshalMessageSetLazyRace
Breakpoint 1 set at 0xac9bf6 for google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500
(dlv) c
=== RUN   TestMarshalMessageSetLazyRace
> [Breakpoint 1] google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (hits goroutine(28):1 total:1) (PC: 0xac9bf6)
=> 500:	func TestMarshalMessageSetLazyRace(t *testing.T) {
(dlv) b ExtensionField.lazyInit
Breakpoint 2 set at 0x8da076 for google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123
(dlv) c
> [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(68):1 total:4) (PC: 0x8da076)
> [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(70):1 total:4) (PC: 0x8da076)
> [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(58):1 total:4) (PC: 0x8da076)
> [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(54):1 total:4) (PC: 0x8da076)
=> 123:	func (f *ExtensionField) lazyInit() {
(dlv) bt
0  0x00000000008da076 in google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit
   at ./codec_extension.go:123
1  0x00000000008dac3e in google.golang.org/protobuf/internal/impl.(*ExtensionField).Value
   at ./codec_extension.go:180
2  0x000000000094e5d9 in google.golang.org/protobuf/internal/impl.(*extensionMap).Get
   at ./message_reflect.go:276
3  0x000000000095a2ee in google.golang.org/protobuf/internal/impl.(*messageState).Get
   at ./message_reflect_gen.go:90
4  0x0000000000890099 in google.golang.org/protobuf/proto.GetExtension
   at /usr/local/google/home/stapelberg/protobuf/proto/extension.go:90
5  0x0000000000aca7d4 in google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace.func2.1
   at ./lazy_test.go:545
6  0x0000000000aca625 in google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace.func2
   at ./lazy_test.go:550
7  0x00000000006001c1 in runtime.goexit
   at /usr/lib/google-golang/src/runtime/asm_amd64.s:1700

Change-Id: Ie7a8621064af412a1db7efb3ad6b8473f9db58e8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/624416
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Chressie Himpel <chressie@google.com>