Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: release 3.20 #2556

Closed
24 tasks done
bassosimone opened this issue Oct 10, 2023 · 0 comments
Closed
24 tasks done

cli: release 3.20 #2556

bassosimone opened this issue Oct 10, 2023 · 0 comments
Assignees

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Oct 10, 2023

Ideally, this release would include an improved Web Connectivity v0.5. If we fail to merge these changes, it would still be nice to release with upgraded dependencies. Supporting go1.21 would also be great, if possible. 🙏

Done

@bassosimone bassosimone self-assigned this Oct 10, 2023
@bassosimone bassosimone mentioned this issue Oct 10, 2023
26 tasks
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 22, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 22, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/oohttp that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/oohttp that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/oocrypto that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/oohttp that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 12, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
Forked off the more complex #1428,
which was failing.

- run `./script/updateminipipeline.bash` 
- run `gofmt -s`
- run `go generate ./...`
- update user agent

See ooni/probe#2556
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
Extracted from #1428 excluding
packages causing tests to fail.

Part of ooni/probe#2556.
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/netem that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/netem that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
Upgrade to [email protected] and adapt closing policy after [changes in
[email protected]](https://github.com/quic-go/quic-go/releases/tag/v0.40.0)
and quic-go/quic-go#4072 in particular. We're
creating an UDP conn and [passing it to
server.Serve](https://github.com/ooni/probe-cli/pull/1428/files#diff-efda3daa51e9aed0b3444a327e64b7e5c412938a1fe894a3c850d533179c2425R105),
which [calls
serveConn](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L242),
which [calls
quicListen](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L316)
and then
[ServeListener](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L321).
In turn, ServeListener [is interrupted by
ErrServerClosed](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L268),
which seems to be generated by [server.Close calling Close for each
listener](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L657).
The following is what happened before updating the shutdown protocol:

```
goroutine 247 [sync.Mutex.Lock]:

[...]

github.com/quic-go/quic-go.(*Transport).closeServer(0x1400054a000?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:298 +0x90

github.com/quic-go/quic-go.(*baseServer).close.func1()
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:344 +0x84

[...]

github.com/quic-go/quic-go.(*baseServer).close(0x14000cc1c10?, {0x102faa7a0?, 0x140002557b0?}, 0xec?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:338 +0x64

github.com/quic-go/quic-go.(*baseServer).Close(...)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:333

github.com/quic-go/quic-go.(*EarlyListener).Close(0x14000120700?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:165 +0x34

github.com/quic-go/quic-go/http3.(*Server).Close(0x140007344d0)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/http3/server.go:657 +0xe8

github.com/ooni/probe-cli/v3/internal/netemx.(*http3Server).Close(0x140001345a0)
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/http3.go:66 +0xe4

github.com/ooni/probe-cli/v3/internal/netemx.(*QAEnv).Close.func1()
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/qaenv.go:291 +0x4c

[...]

github.com/ooni/probe-cli/v3/internal/netemx.(*QAEnv).Close(0x140001e5570?)
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/qaenv.go:288 +0x48

goroutine 136 [sync.Mutex.Lock]:

[...]

github.com/quic-go/quic-go.(*baseServer).close(0x14000127200?, {0x102faa7a0?, 0x140001920f0?}, 0xd8?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:338 +0x64

github.com/quic-go/quic-go.(*Transport).close(0x1400054a000, {0x102faa7a0, 0x140001920f0})
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:325 +0x110

github.com/quic-go/quic-go.(*Transport).listen(0x1400054a000, {0x102fbd3a8, 0x14000526108})
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:358 +0x364

created by github.com/quic-go/quic-go.(*Transport).init.func1
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:242 +0x45c
```

Looking at the above traces, both end up at server.go:338 locking the
same sync.Once. It seems the structures are such that attempting to
close both the server and the listener leads to self locking in
v0.40.{0,1}. The original expectation was that the server closed the
connection used for listening anyway, and
ooni/probe#2527 documented how that was not
the case. It seems that now this is the case, so we can comment out the
original ooni/probe#2527 fix without any test
hangs. Also, if the original bug was indeed that the server did not own
the listener, and considering that now it seems the server owns the
listener, it makes sense that the fix for v0.40.1 is to revert the
ooni/probe#2527 fix.

See ooni/probe#2556
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
We cannot upgrade bifurcation/mint and refraction-networking/utls
because otherwise Psiphon does not compile.

Part of ooni/probe#2556.
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Forked off the more complex ooni#1428,
which was failing.

- run `./script/updateminipipeline.bash` 
- run `gofmt -s`
- run `go generate ./...`
- update user agent

See ooni/probe#2556
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Extracted from ooni#1428 excluding
packages causing tests to fail.

Part of ooni/probe#2556.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Upgrade to [email protected] and adapt closing policy after [changes in
[email protected]](https://github.com/quic-go/quic-go/releases/tag/v0.40.0)
and quic-go/quic-go#4072 in particular. We're
creating an UDP conn and [passing it to
server.Serve](https://github.com/ooni/probe-cli/pull/1428/files#diff-efda3daa51e9aed0b3444a327e64b7e5c412938a1fe894a3c850d533179c2425R105),
which [calls
serveConn](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L242),
which [calls
quicListen](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L316)
and then
[ServeListener](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L321).
In turn, ServeListener [is interrupted by
ErrServerClosed](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L268),
which seems to be generated by [server.Close calling Close for each
listener](https://github.com/quic-go/quic-go/blob/v0.40.1/http3/server.go#L657).
The following is what happened before updating the shutdown protocol:

```
goroutine 247 [sync.Mutex.Lock]:

[...]

github.com/quic-go/quic-go.(*Transport).closeServer(0x1400054a000?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:298 +0x90

github.com/quic-go/quic-go.(*baseServer).close.func1()
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:344 +0x84

[...]

github.com/quic-go/quic-go.(*baseServer).close(0x14000cc1c10?, {0x102faa7a0?, 0x140002557b0?}, 0xec?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:338 +0x64

github.com/quic-go/quic-go.(*baseServer).Close(...)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:333

github.com/quic-go/quic-go.(*EarlyListener).Close(0x14000120700?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:165 +0x34

github.com/quic-go/quic-go/http3.(*Server).Close(0x140007344d0)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/http3/server.go:657 +0xe8

github.com/ooni/probe-cli/v3/internal/netemx.(*http3Server).Close(0x140001345a0)
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/http3.go:66 +0xe4

github.com/ooni/probe-cli/v3/internal/netemx.(*QAEnv).Close.func1()
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/qaenv.go:291 +0x4c

[...]

github.com/ooni/probe-cli/v3/internal/netemx.(*QAEnv).Close(0x140001e5570?)
	/Users/sbs/src/github.com/ooni/probe-cli/internal/netemx/qaenv.go:288 +0x48

goroutine 136 [sync.Mutex.Lock]:

[...]

github.com/quic-go/quic-go.(*baseServer).close(0x14000127200?, {0x102faa7a0?, 0x140001920f0?}, 0xd8?)
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/server.go:338 +0x64

github.com/quic-go/quic-go.(*Transport).close(0x1400054a000, {0x102faa7a0, 0x140001920f0})
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:325 +0x110

github.com/quic-go/quic-go.(*Transport).listen(0x1400054a000, {0x102fbd3a8, 0x14000526108})
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:358 +0x364

created by github.com/quic-go/quic-go.(*Transport).init.func1
	/Users/sbs/go/pkg/mod/github.com/quic-go/[email protected]/transport.go:242 +0x45c
```

Looking at the above traces, both end up at server.go:338 locking the
same sync.Once. It seems the structures are such that attempting to
close both the server and the listener leads to self locking in
v0.40.{0,1}. The original expectation was that the server closed the
connection used for listening anyway, and
ooni/probe#2527 documented how that was not
the case. It seems that now this is the case, so we can comment out the
original ooni/probe#2527 fix without any test
hangs. Also, if the original bug was indeed that the server did not own
the listener, and considering that now it seems the server owns the
listener, it makes sense that the fix for v0.40.1 is to revert the
ooni/probe#2527 fix.

See ooni/probe#2556
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
We cannot upgrade bifurcation/mint and refraction-networking/utls
because otherwise Psiphon does not compile.

Part of ooni/probe#2556.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant