Skip to content

Commit

Permalink
Fix race condition in ErrProtocolNotSupported
Browse files Browse the repository at this point in the history
Fixed a race condition that caused the library (and http proxy) to not
recognize ErrProtocolNotSupported, which indicates the vehicle does not
support the newer protocol.

The race condition was caused by code that attempted to handshake with
multiple vehicle subsystems (domains) in parallel. If one subsystem
returned ErrProtocolNotSupported, the pending requests to other
subsystems were canceled. In some cases, the context cancellation error
would be propagated to the down the stack instead of
ErrProtocolNotSupported.

With this fix, the library prioritizes non-context cancellation errors.
  • Loading branch information
sethterashima committed Jan 12, 2024
1 parent bd191bb commit beb8735
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
10 changes: 7 additions & 3 deletions internal/dispatcher/dispatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dispatcher
import (
"context"
"crypto/rand"
"errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -110,13 +111,16 @@ func (d *Dispatcher) StartSessions(ctx context.Context, domains []universal.Doma
results <- d.StartSession(aggregateContext, dom)
}(domain)
}
var err error
for i := 0; i < len(domains); i++ {
err := <-results
if err != nil {
err = <-results
// The aggregateContext is canceled if one of the handshakes fails. We don't want to return
// the Canceled error if ErrProtocolNotSupported is present.
if !errors.Is(err, context.Canceled) {
return err
}
}
return nil
return err
}

func (d *Dispatcher) createHandler(key *receiverKey) *receiver {
Expand Down
2 changes: 1 addition & 1 deletion pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func (p *Proxy) handleVehicleCommand(acct *account.Account, w http.ResponseWrite
}
defer car.Disconnect()

if err := car.StartSession(ctx, nil); err == protocol.ErrProtocolNotSupported {
if err := car.StartSession(ctx, nil); errors.Is(err, protocol.ErrProtocolNotSupported) {
p.markUnsupportedVIN(vin)
p.forwardRequest(acct.Host, w, req)
return err
Expand Down

0 comments on commit beb8735

Please sign in to comment.