mobile/android: invalid memory address or nil pointer dereference

[bassosimone]

Just seen this crash on Android x86:

2020-02-19 18:30:48.797 10198-0/org.openobservatory.ooniprobe E/Go: panic: runtime error: invalid memory address or nil pointer dereference
2020-02-19 18:30:48.797 10198-0/org.openobservatory.ooniprobe E/Go: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xbc8d617c]
2020-02-19 18:30:48.797 10198-0/org.openobservatory.ooniprobe E/Go: goroutine 103 [running]:
2020-02-19 18:30:48.813 10198-0/org.openobservatory.ooniprobe E/Go: runtime/internal/atomic.Xadd64(0x8c40f604, 0x1, 0x0, 0x1, 0xbc98a401)
2020-02-19 18:30:48.813 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/runtime/internal/atomic/asm_386.s:105 +0xc
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go: github.com/ooni/probe-engine/internal/oonitemplates.(*channelHandler).OnMeasurement(0x8c40f600, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x8c705a40, 0x0, ...)
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/runners/2.165.2/work/probe-engine/probe-engine/internal/oonitemplates/oonitemplates.go:47 +0xef
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go: github.com/ooni/netx/internal/dialer/connx.(*MeasuringConn).Close(0x8c4660f0, 0x8c4805e0, 0x8c480600)
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go:     /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/gomobile-work-589583453/pkg/mod/github.com/ooni/netx@v0.0.0-20200113102411-24ca17149d15/internal/dialer/connx/connx.go:74 +0x1fd
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go: net/http.(*persistConn).closeLocked(0x8c4d0640, 0xbd5dad30, 0x8c4805e0)
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2522 +0xe4
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go: net/http.(*persistConn).close(0x8c4d0640, 0xbd5dad30, 0x8c4805e0)
2020-02-19 18:30:48.814 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2507 +0x76
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go: net/http.(*persistConn).readLoop.func1(0x8c4d0640, 0x8c6d2e9c)
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:1879 +0x33
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go: net/http.(*persistConn).readLoop(0x8c4d0640)
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2055 +0x108a
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go: created by net/http.(*Transport).dialConn
2020-02-19 18:30:48.815 10198-0/org.openobservatory.ooniprobe E/Go:     /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:1580 +0x9e4
2020-02-19 18:30:48.821 10198-10273/org.openobservatory.ooniprobe A/libc: Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 10273 (AsyncTask #1), pid 10198 (atory.ooniprobe)

Of course, this is part of https://github.com/measurement-kit/measurement-kit/issues/1913.

Note that I can't reproduce the crash with android/x86_64. Yet, on android/arm64:

2020-02-19 21:54:53.017 4866-0/? E/Go: panic: runtime error: invalid memory address or nil pointer dereference
2020-02-19 21:54:53.019 4866-0/? E/Go: [signal SIGSEGV: segmentation violation code=0x1 addr=0x4 pc=0x8eff2398]
2020-02-19 21:54:53.024 4866-0/? E/Go: goroutine 142 [running]:
2020-02-19 21:54:53.025 4866-0/? E/Go: github.com/ooni/probe-engine/internal/oonitemplates.(*channelHandler).OnMeasurement(0x50958bd0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x5095ac40, 0x0, ...)
2020-02-19 21:54:53.025 4866-0/? E/Go:  /Users/runner/runners/2.165.2/work/probe-engine/probe-engine/internal/oonitemplates/oonitemplates.go:47 +0xe4
2020-02-19 21:54:53.025 4866-0/? E/Go: github.com/ooni/netx/internal/dialer/connx.(*MeasuringConn).Close(0x50b10030, 0x508105f8, 0x50810618)
2020-02-19 21:54:53.025 4866-0/? E/Go:  /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/gomobile-work-589583453/pkg/mod/github.com/ooni/netx@v0.0.0-20200113102411-24ca17149d15/internal/dialer/connx/connx.go:74 +0x1bc
2020-02-19 21:54:53.026 4866-0/? E/Go: net/http.(*persistConn).closeLocked(0x509e4640, 0x8fce34e0, 0x508105f8)
2020-02-19 21:54:53.026 4866-0/? E/Go:  /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2522 +0xf4
2020-02-19 21:54:53.026 4866-0/? E/Go: net/http.(*persistConn).close(0x509e4640, 0x8fce34e0, 0x508105f8)
2020-02-19 21:54:53.026 4866-0/? E/Go:  /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2507 +0x74
2020-02-19 21:54:53.026 4866-0/? E/Go: net/http.(*persistConn).readLoop.func1(0x509e4640, 0x50c02e9c)
2020-02-19 21:54:53.026 4866-0/? E/Go:  /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:1879 +0x30
2020-02-19 21:54:53.026 4866-0/? E/Go: net/http.(*persistConn).readLoop(0x509e4640)
2020-02-19 21:54:53.027 4866-0/? E/Go:  /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:2055 +0xf44
2020-02-19 21:54:53.027 4866-0/? E/Go: created by net/http.(*Transport).dialConn
2020-02-19 21:54:53.027 4866-0/? E/Go:  /Users/runner/hostedtoolcache/go/1.13.8/x64/src/net/http/transport.go:1580 +0x9a8

[bassosimone]

It is confirmed that the following diff is enough to prevent the crash:

diff --git a/internal/oonitemplates/oonitemplates.go b/internal/oonitemplates/oonitemplates.go
index 80552c2..1701fce 100644
--- a/internal/oonitemplates/oonitemplates.go
+++ b/internal/oonitemplates/oonitemplates.go
@@ -17,7 +17,6 @@ import (
    "net/http"
    "net/url"
    "sync"
-   "sync/atomic"
    "time"

    goptlib "git.torproject.org/pluggable-transports/goptlib.git"
@@ -32,7 +31,6 @@ import (

 type channelHandler struct {
    ch         chan<- modelx.Measurement
-   lateWrites int64
 }

 func (h *channelHandler) OnMeasurement(m modelx.Measurement) {
@@ -44,7 +42,6 @@ func (h *channelHandler) OnMeasurement(m modelx.Measurement) {
    select {
    case h.ch <- m:
    case <-time.After(100 * time.Millisecond):
-       atomic.AddInt64(&h.lateWrites, 1)
    }
 }

diff --git a/internal/oonitemplates/oonitemplates_test.go b/internal/oonitemplates/oonitemplates_test.go
index 797696d..9311cff 100644
--- a/internal/oonitemplates/oonitemplates_test.go
+++ b/internal/oonitemplates/oonitemplates_test.go
@@ -5,33 +5,14 @@ import (
    "errors"
    "net"
    "strings"
-   "sync"
    "testing"
    "time"

    goptlib "git.torproject.org/pluggable-transports/goptlib.git"
-   "github.com/ooni/netx/modelx"
    "gitlab.com/yawning/obfs4.git/transports"
    obfs4base "gitlab.com/yawning/obfs4.git/transports/base"
 )

-func TestUnitChannelHandlerWriteLateOnChannel(t *testing.T) {
-   handler := &channelHandler{
-       ch: make(chan modelx.Measurement),
-   }
-   var waitgroup sync.WaitGroup
-   waitgroup.Add(1)
-   go func() {
-       time.Sleep(1 * time.Second)
-       handler.OnMeasurement(modelx.Measurement{})
-       waitgroup.Done()
-   }()
-   waitgroup.Wait()
-   if handler.lateWrites != 1 {
-       t.Fatal("unexpected lateWrites value")
-   }
-}
-
 func TestIntegrationDNSLookupGood(t *testing.T) {
    ctx := context.Background()
    results := DNSLookup(ctx, DNSLookupConfig{

Now the question is why this happens.

[bassosimone]

This issue has been very useful to teach me facts about github.com/ooni/probe-engine. I have mentioned some of the issues that it inspired above and I am going to mention more issues below.

[bassosimone]

This should now be fixed by https://github.com/ooni/probe-engine/pull/360. This was of course priority/high because it was blocking me from finalising https://github.com/measurement-kit/measurement-kit/issues/1913. This has been effort/L for me, since, in one way or another, it blocked me for ~2 days. But it was also a learning moment and prompted a bunch of follow up issues.

It seems my overall knowledge is such that I cannot determine why the code was wrong at present, which suggests that (1) probably code should be simpler and (2) I may be in the future. Since I have been blocked on this issue for quite some time now, I feel okay to commit the workaround in https://github.com/ooni/probe-engine/pull/360 to master and finish with experimenting with the ooni/probe-android app to see how to sketch out integrating it with Go code.

[bassosimone]

For completeness, this was actually https://github.com/ooni/probe-engine/issues/399.