From 81f61b0641cfb30661f7f05a9e75f3b6554a451c Mon Sep 17 00:00:00 2001 From: Kristina Patrikson Date: Mon, 5 Feb 2024 16:45:20 +0100 Subject: [PATCH 1/3] Add ConnOption WithDefaultSendTimeout Added a default timeout for a connection, as well as a ConnOption to set this value. If a default timeout has been set, all calls from that connection will be aborted if the timeout is exceeded. If we always want to have the same timeout for all calls from a connection, it would be easier if the timeout could be set once to the connection itself rather than always having to use CallWithContext. Change-Id: I49f34caeeace5b2cf29e56ef97f7a4817d15d158 --- conn.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/conn.go b/conn.go index bbe111b2..a9ec32ea 100644 --- a/conn.go +++ b/conn.go @@ -7,6 +7,7 @@ import ( "os" "strings" "sync" + "time" ) var ( @@ -30,8 +31,9 @@ var ErrClosed = errors.New("dbus: connection closed by user") type Conn struct { transport - ctx context.Context - cancelCtx context.CancelFunc + ctx context.Context + cancelCtx context.CancelFunc + defaultTimeout time.Duration closeOnce sync.Once closeErr error @@ -258,6 +260,14 @@ func WithContext(ctx context.Context) ConnOption { } } +// WithDeafultTimeout supplies the connection with a default timeout to outgoing messages. +func WithDefaultSendTimeout(timeout time.Duration) ConnOption { + return func(conn *Conn) error { + conn.defaultTimeout = timeout + return nil + } +} + // NewConn creates a new private *Conn from an already established connection. func NewConn(conn io.ReadWriteCloser, opts ...ConnOption) (*Conn, error) { return newConn(genericTransport{conn}, opts...) @@ -540,7 +550,12 @@ func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call) *Call { } var call *Call - ctx, canceler := context.WithCancel(ctx) + var canceler context.CancelFunc + if conn.defaultTimeout > 0 { + ctx, canceler = context.WithTimeout(context.Background(), conn.defaultTimeout) + } else { + ctx, canceler = context.WithCancel(ctx) + } msg.serial = conn.getSerial() if msg.Type == TypeMethodCall && msg.Flags&FlagNoReplyExpected == 0 { call = new(Call) From 11bc204cbee373947235134e0e975b4662954809 Mon Sep 17 00:00:00 2001 From: Kristina Patrikson Date: Mon, 5 Feb 2024 16:45:20 +0100 Subject: [PATCH 2/3] Add ConnOption WithDefaultSendTimeout Added a default timeout for a connection, as well as a ConnOption to set this value. If a default timeout has been set, all calls from that connection will be aborted if the timeout is exceeded unless we have specified the context. If we always want to have the same timeout for all calls from a connection, it would be easier if the timeout could be set once to the connection itself rather than always having to use CallWithContext. Change-Id: I49f34caeeace5b2cf29e56ef97f7a4817d15d158 --- conn.go | 27 +++++++++++++++++++++------ object.go | 15 +++++++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/conn.go b/conn.go index bbe111b2..1e024e53 100644 --- a/conn.go +++ b/conn.go @@ -7,6 +7,7 @@ import ( "os" "strings" "sync" + "time" ) var ( @@ -30,8 +31,9 @@ var ErrClosed = errors.New("dbus: connection closed by user") type Conn struct { transport - ctx context.Context - cancelCtx context.CancelFunc + ctx context.Context + cancelCtx context.CancelFunc + defaultTimeout time.Duration closeOnce sync.Once closeErr error @@ -258,6 +260,14 @@ func WithContext(ctx context.Context) ConnOption { } } +// WithDeafultSendTimeout supplies the connection with a default timeout for outgoing messages. +func WithDefaultSendTimeout(timeout time.Duration) ConnOption { + return func(conn *Conn) error { + conn.defaultTimeout = timeout + return nil + } +} + // NewConn creates a new private *Conn from an already established connection. func NewConn(conn io.ReadWriteCloser, opts ...ConnOption) (*Conn, error) { return newConn(genericTransport{conn}, opts...) @@ -521,15 +531,15 @@ func (conn *Conn) handleSendError(msg *Message, err error) { // once the call is complete. Otherwise, ch is ignored and a Call structure is // returned of which only the Err member is valid. func (conn *Conn) Send(msg *Message, ch chan *Call) *Call { - return conn.send(context.Background(), msg, ch) + return conn.send(context.Background(), msg, ch, false) } // SendWithContext acts like Send but takes a context func (conn *Conn) SendWithContext(ctx context.Context, msg *Message, ch chan *Call) *Call { - return conn.send(ctx, msg, ch) + return conn.send(ctx, msg, ch, true) } -func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call) *Call { +func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call, withContext bool) *Call { if ctx == nil { panic("nil context") } @@ -540,7 +550,12 @@ func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call) *Call { } var call *Call - ctx, canceler := context.WithCancel(ctx) + var canceler context.CancelFunc + if !withContext && conn.defaultTimeout > 0 { + ctx, canceler = context.WithTimeout(context.Background(), conn.defaultTimeout) + } else { + ctx, canceler = context.WithCancel(ctx) + } msg.serial = conn.getSerial() if msg.Type == TypeMethodCall && msg.Flags&FlagNoReplyExpected == 0 { call = new(Call) diff --git a/object.go b/object.go index b4b1e939..91ff6ce9 100644 --- a/object.go +++ b/object.go @@ -31,12 +31,12 @@ type Object struct { // Call calls a method with (*Object).Go and waits for its reply. func (o *Object) Call(method string, flags Flags, args ...interface{}) *Call { - return <-o.createCall(context.Background(), method, flags, make(chan *Call, 1), args...).Done + return <-o.createCall(context.Background(), method, flags, make(chan *Call, 1), false, args...).Done } // CallWithContext acts like Call but takes a context func (o *Object) CallWithContext(ctx context.Context, method string, flags Flags, args ...interface{}) *Call { - return <-o.createCall(ctx, method, flags, make(chan *Call, 1), args...).Done + return <-o.createCall(ctx, method, flags, make(chan *Call, 1), true, args...).Done } // AddMatchSignal subscribes BusObject to signals from specified interface, @@ -90,15 +90,15 @@ func (o *Object) RemoveMatchSignal(iface, member string, options ...MatchOption) // If the method parameter contains a dot ('.'), the part before the last dot // specifies the interface on which the method is called. func (o *Object) Go(method string, flags Flags, ch chan *Call, args ...interface{}) *Call { - return o.createCall(context.Background(), method, flags, ch, args...) + return o.createCall(context.Background(), method, flags, ch, false, args...) } // GoWithContext acts like Go but takes a context func (o *Object) GoWithContext(ctx context.Context, method string, flags Flags, ch chan *Call, args ...interface{}) *Call { - return o.createCall(ctx, method, flags, ch, args...) + return o.createCall(ctx, method, flags, ch, true, args...) } -func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch chan *Call, args ...interface{}) *Call { +func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch chan *Call, withContext bool, args ...interface{}) *Call { if ctx == nil { panic("nil context") } @@ -122,7 +122,10 @@ func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch if len(args) > 0 { msg.Headers[FieldSignature] = MakeVariant(SignatureOf(args...)) } - return o.conn.SendWithContext(ctx, msg, ch) + if withContext { + return o.conn.SendWithContext(ctx, msg, ch) + } + return o.conn.Send(msg, ch) } // GetProperty calls org.freedesktop.DBus.Properties.Get on the given From 8424481fe0f39567d004495a55fbe032804bfc33 Mon Sep 17 00:00:00 2001 From: Kristina Patrikson Date: Wed, 7 Feb 2024 16:23:51 +0100 Subject: [PATCH 3/3] Call and Go use context.TODO() instead of context.Background() Instead of using a bool to determine if we have specified the context, we now assume that context.TODO() is only sent from functions where the user didn't specify a context. If we have context.TODO() and a default timeout we will use that to create our context and cancelFunc, otherwise we will use context.Background() or the provided context. Change-Id: I043aae124a92ba55beac0f380dc8ad3a44bbf382 --- conn.go | 16 ++++++++++------ object.go | 15 ++++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/conn.go b/conn.go index 1e024e53..ab81c8ad 100644 --- a/conn.go +++ b/conn.go @@ -260,7 +260,7 @@ func WithContext(ctx context.Context) ConnOption { } } -// WithDeafultSendTimeout supplies the connection with a default timeout for outgoing messages. +// WithDefaultSendTimeout supplies the connection with a default timeout for outgoing messages. func WithDefaultSendTimeout(timeout time.Duration) ConnOption { return func(conn *Conn) error { conn.defaultTimeout = timeout @@ -531,15 +531,15 @@ func (conn *Conn) handleSendError(msg *Message, err error) { // once the call is complete. Otherwise, ch is ignored and a Call structure is // returned of which only the Err member is valid. func (conn *Conn) Send(msg *Message, ch chan *Call) *Call { - return conn.send(context.Background(), msg, ch, false) + return conn.send(context.Background(), msg, ch) } // SendWithContext acts like Send but takes a context func (conn *Conn) SendWithContext(ctx context.Context, msg *Message, ch chan *Call) *Call { - return conn.send(ctx, msg, ch, true) + return conn.send(ctx, msg, ch) } -func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call, withContext bool) *Call { +func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call) *Call { if ctx == nil { panic("nil context") } @@ -551,8 +551,12 @@ func (conn *Conn) send(ctx context.Context, msg *Message, ch chan *Call, withCon var call *Call var canceler context.CancelFunc - if !withContext && conn.defaultTimeout > 0 { - ctx, canceler = context.WithTimeout(context.Background(), conn.defaultTimeout) + if ctx == context.TODO() { + if conn.defaultTimeout > 0 { + ctx, canceler = context.WithTimeout(context.Background(), conn.defaultTimeout) + } else { + ctx, canceler = context.WithCancel(context.Background()) + } } else { ctx, canceler = context.WithCancel(ctx) } diff --git a/object.go b/object.go index 91ff6ce9..e8ae640d 100644 --- a/object.go +++ b/object.go @@ -31,12 +31,12 @@ type Object struct { // Call calls a method with (*Object).Go and waits for its reply. func (o *Object) Call(method string, flags Flags, args ...interface{}) *Call { - return <-o.createCall(context.Background(), method, flags, make(chan *Call, 1), false, args...).Done + return <-o.createCall(context.TODO(), method, flags, make(chan *Call, 1), args...).Done } // CallWithContext acts like Call but takes a context func (o *Object) CallWithContext(ctx context.Context, method string, flags Flags, args ...interface{}) *Call { - return <-o.createCall(ctx, method, flags, make(chan *Call, 1), true, args...).Done + return <-o.createCall(ctx, method, flags, make(chan *Call, 1), args...).Done } // AddMatchSignal subscribes BusObject to signals from specified interface, @@ -90,15 +90,15 @@ func (o *Object) RemoveMatchSignal(iface, member string, options ...MatchOption) // If the method parameter contains a dot ('.'), the part before the last dot // specifies the interface on which the method is called. func (o *Object) Go(method string, flags Flags, ch chan *Call, args ...interface{}) *Call { - return o.createCall(context.Background(), method, flags, ch, false, args...) + return o.createCall(context.TODO(), method, flags, ch, args...) } // GoWithContext acts like Go but takes a context func (o *Object) GoWithContext(ctx context.Context, method string, flags Flags, ch chan *Call, args ...interface{}) *Call { - return o.createCall(ctx, method, flags, ch, true, args...) + return o.createCall(ctx, method, flags, ch, args...) } -func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch chan *Call, withContext bool, args ...interface{}) *Call { +func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch chan *Call, args ...interface{}) *Call { if ctx == nil { panic("nil context") } @@ -122,10 +122,7 @@ func (o *Object) createCall(ctx context.Context, method string, flags Flags, ch if len(args) > 0 { msg.Headers[FieldSignature] = MakeVariant(SignatureOf(args...)) } - if withContext { - return o.conn.SendWithContext(ctx, msg, ch) - } - return o.conn.Send(msg, ch) + return o.conn.SendWithContext(ctx, msg, ch) } // GetProperty calls org.freedesktop.DBus.Properties.Get on the given