From 909c9c7ee7c347c7decd3b103bb78aed9886f226 Mon Sep 17 00:00:00 2001 From: Ulises Jeremias Cornejo Fandos Date: Tue, 13 Apr 2021 01:04:13 -0300 Subject: [PATCH] context: small refactor to always use Context type instead of multiple types (#9705) --- cmd/tools/vtest-self.v | 1 + vlib/context/README.md | 36 +++++++++--------- vlib/context/_context.v | 4 +- vlib/context/cancel.v | 74 +++--------------------------------- vlib/context/cancel_test.v | 21 +++++----- vlib/context/deadline.v | 10 ++--- vlib/context/deadline_test.v | 63 ++++++++++++++++++++++++++++++ vlib/context/value.v | 5 +-- vlib/context/value_test.v | 2 +- 9 files changed, 108 insertions(+), 108 deletions(-) create mode 100644 vlib/context/deadline_test.v diff --git a/cmd/tools/vtest-self.v b/cmd/tools/vtest-self.v index 7b7295cba2..ed6cee9f01 100644 --- a/cmd/tools/vtest-self.v +++ b/cmd/tools/vtest-self.v @@ -58,6 +58,7 @@ const ( 'vlib/context/value_test.v' /* the following tests need C casts in `sync` and/or thirdparty/stdatomic */, 'vlib/context/empty_test.v', 'vlib/context/cancel_test.v', + 'vlib/context/deadline_test.v', 'vlib/sync/array_rlock_test.v', 'vlib/sync/atomic2/atomic_test.v', 'vlib/sync/channel_2_test.v', diff --git a/vlib/context/README.md b/vlib/context/README.md index 54eedb517f..fb1b02467b 100644 --- a/vlib/context/README.md +++ b/vlib/context/README.md @@ -10,10 +10,9 @@ with_deadline, with_timeout, or with_value. When a Context is canceled, all Cont derived from it are also canceled. The with_cancel, with_deadline, and with_timeout functions take a Context (the parent) -and return a derived Context (the child) and a CancelFunc. Calling the CancelFunc +and return a derived Context (the child). Calling the cancel function cancels the child and its children, removes the parent's reference to the child, -and stops any associated timers. Failing to call the CancelFunc leaks the child -and its children until the parent is canceled or the timer fires. +and stops any associated timers. Programs that use Contexts should follow these rules to keep interfaces consistent across different modules. @@ -40,29 +39,32 @@ fn example_with_cancel() { // The callers of gen need to cancel the context once // they are done consuming generated integers not to leak // the internal routine started by gen. - gen := fn (mut ctx context.CancelerContext) chan int { + gen := fn (ctx context.Context) chan int { dst := chan int{} - go fn (mut ctx context.CancelerContext, dst chan int) { + go fn (ctx context.Context, dst chan int) { + mut v := 0 ch := ctx.done() - loop: for i in 0 .. 5 { + for { select { _ := <-ch { // returning not to leak the routine - break loop + return + } + dst <- v { + v++ } - dst <- i {} } } - }(mut ctx, dst) + }(ctx, dst) return dst } - mut ctx := context.with_cancel(context.background()) + ctx := context.with_cancel(context.background()) defer { - context.cancel(mut ctx) + context.cancel(ctx) } - ch := gen(mut ctx) + ch := gen(ctx) for i in 0 .. 5 { v := <-ch assert i == v @@ -94,13 +96,13 @@ fn after(dur time.Duration) chan int { // function that it should abandon its work as soon as it gets to it. fn example_with_deadline() { dur := time.now().add(short_duration) - mut ctx := context.with_deadline(context.background(), dur) + ctx := context.with_deadline(context.background(), dur) defer { // Even though ctx will be expired, it is good practice to call its // cancellation function in any case. Failure to do so may keep the // context and its parent alive longer than necessary. - context.cancel(mut ctx) + context.cancel(ctx) } after_ch := after(1 * time.second) @@ -141,9 +143,9 @@ fn after(dur time.Duration) chan int { fn example_with_timeout() { // Pass a context with a timeout to tell a blocking function that it // should abandon its work after the timeout elapses. - mut ctx := context.with_timeout(context.background(), short_duration) + ctx := context.with_timeout(context.background(), short_duration) defer { - context.cancel(mut ctx) + context.cancel(ctx) } after_ch := after(1 * time.second) @@ -169,7 +171,7 @@ type ValueContextKey = string // This example demonstrates how a value can be passed to the context // and also how to retrieve it if it exists. fn example_with_value() { - f := fn (ctx context.ValueContext, key ValueContextKey) string { + f := fn (ctx context.Context, key ValueContextKey) string { if value := ctx.value(key) { if !isnil(value) { return *(&string(value)) diff --git a/vlib/context/_context.v b/vlib/context/_context.v index 7675114f4d..b7f20c2705 100644 --- a/vlib/context/_context.v +++ b/vlib/context/_context.v @@ -61,7 +61,7 @@ pub interface Context { // initialization, and tests, and as the top-level Context for incoming // requests. pub fn background() Context { - return Context(context.background) + return context.background } // todo returns an empty Context. Code should use todo when @@ -69,7 +69,7 @@ pub fn background() Context { // surrounding function has not yet been extended to accept a Context // parameter). pub fn todo() Context { - return Context(context.todo) + return context.todo } fn context_name(ctx Context) string { diff --git a/vlib/context/cancel.v b/vlib/context/cancel.v index 7a467054ea..5d7654fc5e 100644 --- a/vlib/context/cancel.v +++ b/vlib/context/cancel.v @@ -8,10 +8,9 @@ pub interface Canceler { id string cancel(remove_from_parent bool, err string) done() chan int - str() string } -pub fn cancel(mut ctx CancelerContext) { +pub fn cancel(ctx Context) { match mut ctx { CancelContext { ctx.cancel(true, canceled) @@ -19,65 +18,7 @@ pub fn cancel(mut ctx CancelerContext) { TimerContext { ctx.cancel(true, canceled) } - } -} - -// CancelerContext implements the Canceler intarface for both -// struct types: CancelContext and TimerContext -pub type CancelerContext = CancelContext | TimerContext - -pub fn (mut ctx CancelerContext) done() chan int { - match mut ctx { - CancelContext { - return ctx.done() - } - TimerContext { - return ctx.done() - } - } -} - -pub fn (mut ctx CancelerContext) err() string { - match mut ctx { - CancelContext { - return ctx.err() - } - TimerContext { - return ctx.err() - } - } -} - -pub fn (ctx CancelerContext) value(key string) ?voidptr { - match ctx { - CancelContext { - return ctx.value(key) - } - TimerContext { - return ctx.value(key) - } - } -} - -pub fn (mut ctx CancelerContext) cancel(remove_from_parent bool, err string) { - match mut ctx { - CancelContext { - ctx.cancel(remove_from_parent, err) - } - TimerContext { - ctx.cancel(remove_from_parent, err) - } - } -} - -pub fn (ctx CancelerContext) str() string { - match ctx { - CancelContext { - return ctx.str() - } - TimerContext { - return ctx.str() - } + else {} } } @@ -93,22 +34,16 @@ mut: err string } -// A CancelFunc tells an operation to abandon its work. -// A CancelFunc does not wait for the work to stop. -// A CancelFunc may be called by multiple goroutines simultaneously. -// After the first call, subsequent calls to a CancelFunc do nothing. -// pub type CancelFunc = fn (c Canceler) - // with_cancel returns a copy of parent with a new done channel. The returned // context's done channel is closed when the returned cancel function is called // or when the parent context's done channel is closed, whichever happens first. // // Canceling this context releases resources associated with it, so code should // call cancel as soon as the operations running in this Context complete. -pub fn with_cancel(parent Context) &CancelerContext { +pub fn with_cancel(parent Context) Context { mut c := new_cancel_context(parent) propagate_cancel(parent, mut c) - return c + return Context(c) } // new_cancel_context returns an initialized CancelContext. @@ -164,6 +99,7 @@ fn (mut ctx CancelContext) cancel(remove_from_parent bool, err string) { ctx.err = err if !ctx.done.closed { + ctx.done <- 0 ctx.done.close() } diff --git a/vlib/context/cancel_test.v b/vlib/context/cancel_test.v index be99cd067f..6b9fdaabe0 100644 --- a/vlib/context/cancel_test.v +++ b/vlib/context/cancel_test.v @@ -9,29 +9,32 @@ fn test_with_cancel() { // The callers of gen need to cancel the context once // they are done consuming generated integers not to leak // the internal routine started by gen. - gen := fn (mut ctx context.CancelerContext) chan int { + gen := fn (ctx context.Context) chan int { dst := chan int{} - go fn (mut ctx context.CancelerContext, dst chan int) { + go fn (ctx context.Context, dst chan int) { + mut v := 0 ch := ctx.done() - loop: for i in 0 .. 5 { + for { select { _ := <-ch { // returning not to leak the routine - break loop + return + } + dst <- v { + v++ } - dst <- i {} } } - }(mut ctx, dst) + }(ctx, dst) return dst } - mut ctx := context.with_cancel(context.background()) + ctx := context.with_cancel(context.background()) defer { - context.cancel(mut ctx) + context.cancel(ctx) } - ch := gen(mut ctx) + ch := gen(ctx) for i in 0 .. 5 { v := <-ch assert i == v diff --git a/vlib/context/deadline.v b/vlib/context/deadline.v index c115a90490..f67de09f66 100644 --- a/vlib/context/deadline.v +++ b/vlib/context/deadline.v @@ -22,7 +22,7 @@ mut: // // Canceling this context releases resources associated with it, so code should // call cancel as soon as the operations running in this Context complete. -pub fn with_deadline(parent Context, d time.Time) &CancelerContext { +pub fn with_deadline(parent Context, d time.Time) Context { id := rand.uuid_v4() if cur := parent.deadline() { if cur < d { @@ -40,25 +40,23 @@ pub fn with_deadline(parent Context, d time.Time) &CancelerContext { dur := d - time.now() if dur.nanoseconds() <= 0 { ctx.cancel(true, deadline_exceeded) // deadline has already passed - return ctx + return Context(ctx) } if ctx.cancel_ctx.err() == '' { go fn (mut ctx TimerContext, dur time.Duration) { time.sleep(dur) - ctx_ch := ctx.done() - ctx_ch <- 0 ctx.cancel(true, deadline_exceeded) }(mut ctx, dur) } - return ctx + return Context(ctx) } // with_timeout returns with_deadline(parent, time.now().add(timeout)). // // Canceling this context releases resources associated with it, so code should // call cancel as soon as the operations running in this Context complete -pub fn with_timeout(parent Context, timeout time.Duration) &CancelerContext { +pub fn with_timeout(parent Context, timeout time.Duration) Context { return with_deadline(parent, time.now().add(timeout)) } diff --git a/vlib/context/deadline_test.v b/vlib/context/deadline_test.v new file mode 100644 index 0000000000..0f052e5112 --- /dev/null +++ b/vlib/context/deadline_test.v @@ -0,0 +1,63 @@ +import context +import time + +const ( + // a reasonable duration to block in an example + short_duration = 1 * time.millisecond +) + +fn after(dur time.Duration) chan int { + dst := chan int{} + go fn (dur time.Duration, dst chan int) { + time.sleep(dur) + dst <- 0 + }(dur, dst) + return dst +} + +// This example passes a context with an arbitrary deadline to tell a blocking +// function that it should abandon its work as soon as it gets to it. +fn test_with_deadline() { + dur := time.now().add(short_duration) + ctx := context.with_deadline(context.background(), dur) + + defer { + // Even though ctx will be expired, it is good practice to call its + // cancellation function in any case. Failure to do so may keep the + // context and its parent alive longer than necessary. + context.cancel(ctx) + } + + after_ch := after(1 * time.second) + ctx_ch := ctx.done() + select { + _ := <-after_ch { + assert false + } + _ := <-ctx_ch { + assert true + } + } +} + +// This example passes a context with a timeout to tell a blocking function that +// it should abandon its work after the timeout elapses. +fn test_with_timeout() { + // Pass a context with a timeout to tell a blocking function that it + // should abandon its work after the timeout elapses. + ctx := context.with_timeout(context.background(), short_duration) + defer { + context.cancel(ctx) + } + + after_ch := after(1 * time.second) + ctx_ch := ctx.done() + select { + _ := <-after_ch { + assert false + } + _ := <-ctx_ch { + assert true + } + } +} diff --git a/vlib/context/value.v b/vlib/context/value.v index de314fabcf..a5da6b575d 100644 --- a/vlib/context/value.v +++ b/vlib/context/value.v @@ -21,10 +21,7 @@ mut: // string or any other built-in type to avoid collisions between // packages using context. Users of with_value should define their own // types for keys -pub fn with_value(parent Context, key string, value voidptr) &ValueContext { - if isnil(key) { - panic('nil key') - } +pub fn with_value(parent Context, key string, value voidptr) Context { return &ValueContext{ context: parent key: key diff --git a/vlib/context/value_test.v b/vlib/context/value_test.v index d10b5486ea..a8ed5b5232 100644 --- a/vlib/context/value_test.v +++ b/vlib/context/value_test.v @@ -5,7 +5,7 @@ type ValueContextKey = string // This example demonstrates how a value can be passed to the context // and also how to retrieve it if it exists. fn test_with_value() { - f := fn (ctx context.ValueContext, key ValueContextKey) string { + f := fn (ctx context.Context, key ValueContextKey) string { if value := ctx.value(key) { if !isnil(value) { return *(&string(value))