ref: Prevent crashes when advertising >16 syncgroups
Currently two related crashes will occur should the user have more
than 16 syncgroups. The root of the cause is Serve() will start
advertising each syncgroup, and if the platform has BLE then
some will fail as the current BLE implementation can only support
16 total advertisements. If BLE advertisement fails, one crash was
a null dereference of the advertisement's stop(). Note CL #23804
fixed this type of crash in one place in the function there was
another where stop could be nil and dereferenced.
The other crash is a vlog.Fatal should advertising syncgroups fail. This
CL allows advertising to continue (only logging errors) should at least
one plugin work. That way BLE can work up to its max, but mDNS can
continue advertising all syncgroups. This is one solve the >16 BLE
advertisements issue with BLE, filed as vanadium/issues#1404.
Closes vanadium/issues#1390, vanadium/issues#1403, vanadium/issues#1398
Change-Id: I07fee516a5246ad8ff72e1107158c16f33d68562
diff --git a/lib/discovery/advertise.go b/lib/discovery/advertise.go
index 67d43cb..a6a2ed0 100644
--- a/lib/discovery/advertise.go
+++ b/lib/discovery/advertise.go
@@ -13,6 +13,7 @@
"v.io/v23/discovery"
"v.io/v23/naming"
"v.io/v23/security"
+ "v.io/x/lib/vlog"
"v.io/x/ref/lib/stats"
)
@@ -121,13 +122,24 @@
stats.NewStringFunc(statName, func() string { return fmt.Sprint(*adinfo) })
ctx, cancel := context.WithCancel(ctx)
var wg sync.WaitGroup
+ var lastErr error
+ isAdvertising := false
for _, plugin := range d.plugins {
wg.Add(1)
if err := plugin.Advertise(ctx, adinfo, wg.Done); err != nil {
- cancel()
- return nil, err
+ // Only log errors advertising just as long as it succeeds in at least one plugin.
+ // See https://github.com/vanadium/issues/issues/1404 for discussion on issues
+ // with more than 16 BLE advertisements.
+ vlog.Error("discovery: Unable to startAdvertisement for plugin: ", err)
+ lastErr = err
+ } else {
+ isAdvertising = true
}
}
+ if !isAdvertising {
+ cancel()
+ return nil, lastErr
+ }
stop := func() {
stats.Delete(statName)
diff --git a/lib/discovery/advertise_server.go b/lib/discovery/advertise_server.go
index 206a170..85cedaf 100644
--- a/lib/discovery/advertise_server.go
+++ b/lib/discovery/advertise_server.go
@@ -60,7 +60,6 @@
stop() // Stop the previous advertisement.
stop, err = advertiseServer(ctx, d, ad, newAddrs, suffix, visibility)
if err != nil {
- stop()
ctx.Error(err)
return
}
