Skip to content

Commit

Permalink
rpc: remove NotifierContextKey
Browse files Browse the repository at this point in the history
Context keys must have a unique type in order to prevent
any unintented clashes. The code used int(1) as key.

Fix it by implementing the pattern recommended by package context.
  • Loading branch information
fjl committed Apr 15, 2016
1 parent 6197fbf commit a40e61b
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 12 deletions.
4 changes: 2 additions & 2 deletions eth/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ type NewBlocksArgs struct {
// NewBlocks triggers a new block event each time a block is appended to the chain. It accepts an argument which allows
// the caller to specify whether the output should contain transactions and in what format.
func (s *PublicBlockChainAPI) NewBlocks(ctx context.Context, args NewBlocksArgs) (rpc.Subscription, error) {
notifier, supported := ctx.Value(rpc.NotifierContextKey).(rpc.Notifier)
notifier, supported := rpc.NotifierFromContext(ctx)
if !supported {
return nil, rpc.ErrNotificationsUnsupported
}
Expand Down Expand Up @@ -1345,7 +1345,7 @@ func (s *PublicTransactionPoolAPI) PendingTransactions() []*RPCTransaction {
// NewPendingTransaction creates a subscription that is triggered each time a transaction enters the transaction pool
// and is send from one of the transactions this nodes manages.
func (s *PublicTransactionPoolAPI) NewPendingTransactions(ctx context.Context) (rpc.Subscription, error) {
notifier, supported := ctx.Value(rpc.NotifierContextKey).(rpc.Notifier)
notifier, supported := rpc.NotifierFromContext(ctx)
if !supported {
return nil, rpc.ErrNotificationsUnsupported
}
Expand Down
2 changes: 1 addition & 1 deletion eth/downloader/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type SyncingResult struct {

// Syncing provides information when this nodes starts synchronising with the Ethereum network and when it's finished.
func (api *PublicDownloaderAPI) Syncing(ctx context.Context) (rpc.Subscription, error) {
notifier, supported := ctx.Value(rpc.NotifierContextKey).(rpc.Notifier)
notifier, supported := rpc.NotifierFromContext(ctx)
if !supported {
return nil, rpc.ErrNotificationsUnsupported
}
Expand Down
2 changes: 1 addition & 1 deletion eth/filters/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (s *PublicFilterAPI) newLogFilter(earliest, latest int64, addresses []commo
}

func (s *PublicFilterAPI) Logs(ctx context.Context, args NewFilterArgs) (rpc.Subscription, error) {
notifier, supported := ctx.Value(rpc.NotifierContextKey).(rpc.Notifier)
notifier, supported := rpc.NotifierFromContext(ctx)
if !supported {
return nil, rpc.ErrNotificationsUnsupported
}
Expand Down
9 changes: 9 additions & 0 deletions rpc/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/ethereum/go-ethereum/logger"
"github.com/ethereum/go-ethereum/logger/glog"
"golang.org/x/net/context"
)

var (
Expand Down Expand Up @@ -62,6 +63,14 @@ type Notifier interface {
Unsubscribe(id string) error
}

type notifierKey struct{}

// NotifierFromContext returns the Notifier value stored in ctx, if any.
func NotifierFromContext(ctx context.Context) (Notifier, bool) {
n, ok := ctx.Value(notifierKey{}).(Notifier)
return n, ok
}

// Subscription defines the interface for objects that can notify subscribers
type Subscription interface {
// Inform client of an event
Expand Down
2 changes: 1 addition & 1 deletion rpc/notification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (s *NotificationTestService) Unsubscribe(subid string) {
}

func (s *NotificationTestService) SomeSubscription(ctx context.Context, n, val int) (Subscription, error) {
notifier, supported := ctx.Value(NotifierContextKey).(Notifier)
notifier, supported := NotifierFromContext(ctx)
if !supported {
return nil, ErrNotificationsUnsupported
}
Expand Down
11 changes: 4 additions & 7 deletions rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ import (
const (
stopPendingRequestTimeout = 3 * time.Second // give pending requests stopPendingRequestTimeout the time to finish when the server is stopped

// NotifierContextKey is the key where the notifier associated with the codec is stored in the context
NotifierContextKey = 1

notificationBufferSize = 10000 // max buffered notifications before codec is closed

DefaultIPCApis = "admin,eth,debug,miner,net,shh,txpool,personal,web3"
Expand Down Expand Up @@ -171,7 +168,7 @@ func (s *Server) serveRequest(codec ServerCodec, singleShot bool, options CodecO
// to send notification to clients. It is thight to the codec/connection. If the
// connection is closed the notifier will stop and cancels all active subscriptions.
if options&OptionSubscriptions == OptionSubscriptions {
ctx = context.WithValue(ctx, NotifierContextKey, newBufferedNotifier(codec, notificationBufferSize))
ctx = context.WithValue(ctx, notifierKey{}, newBufferedNotifier(codec, notificationBufferSize))
}
s.codecsMu.Lock()
if atomic.LoadInt32(&s.run) != 1 { // server stopped
Expand Down Expand Up @@ -275,7 +272,7 @@ func (s *Server) handle(ctx context.Context, codec ServerCodec, req *serverReque

if req.isUnsubscribe { // cancel subscription, first param must be the subscription id
if len(req.args) >= 1 && req.args[0].Kind() == reflect.String {
notifier, supported := ctx.Value(NotifierContextKey).(*bufferedNotifier)
notifier, supported := NotifierFromContext(ctx)
if !supported { // interface doesn't support subscriptions (e.g. http)
return codec.CreateErrorResponse(&req.id, &callbackError{ErrNotificationsUnsupported.Error()}), nil
}
Expand All @@ -298,8 +295,8 @@ func (s *Server) handle(ctx context.Context, codec ServerCodec, req *serverReque

// active the subscription after the sub id was successful sent to the client
activateSub := func() {
notifier, _ := ctx.Value(NotifierContextKey).(*bufferedNotifier)
notifier.activate(subid)
notifier, _ := NotifierFromContext(ctx)
notifier.(*bufferedNotifier).activate(subid)
}

return codec.CreateResponse(req.id, subid), activateSub
Expand Down

0 comments on commit a40e61b

Please sign in to comment.