diff options
-rw-r--r-- | doc/core/grpc-error.md | 160 | ||||
-rw-r--r-- | src/core/lib/iomgr/error.h | 25 | ||||
-rw-r--r-- | tools/doxygen/Doxyfile.core | 1 | ||||
-rw-r--r-- | tools/doxygen/Doxyfile.core.internal | 1 |
4 files changed, 165 insertions, 22 deletions
diff --git a/doc/core/grpc-error.md b/doc/core/grpc-error.md new file mode 100644 index 0000000000..1fa2480231 --- /dev/null +++ b/doc/core/grpc-error.md @@ -0,0 +1,160 @@ +# gRPC Error + +## Background + +`grpc_error` is the c-core's opaque representation of an error. It holds a +collection of integers, strings, timestamps, and child errors that related to +the final error. + +always present are: + +* GRPC_ERROR_STR_FILE and GRPC_ERROR_INT_FILE_LINE - the source location where + the error was generated +* GRPC_ERROR_STR_DESCRIPTION - a human readable description of the error +* GRPC_ERROR_TIME_CREATED - a timestamp indicating when the error happened + +An error can also have children; these are other errors that are believed to +have contributed to this one. By accumulating children, we can begin to root +cause high level failures from low level failures, without having to derive +execution paths from log lines. + +grpc_errors are refcounted objects, which means they need strict ownership +semantics. An extra ref on an error can cause a memory leak, and a missing ref +can cause a crash. + +This document serves as a detailed overview of grpc_error's ownership rules. It +should help people use the errors, as well as help people debug refcount related +errors. + +## Clarification of Ownership + +If a particular function is said to "own" an error, that means it has the +responsibility of calling unref on the error. A function may have access to an +error without ownership of it. + +This means the function may use the error, but must not call unref on it, since +that will be done elsewhere in the code. A function that does not own an error +may explicitly take ownership of it by manually calling GRPC_ERROR_REF. + +## Ownership Rules + +There are three rules of error ownership, which we will go over in detail. + +* If `grpc_error` is returned by a function, the caller owns a ref to that + instance. +* If a `grpc_error` is passed to a `grpc_closure` callback function, then that + function does not own a ref to the error. +* if a `grpc_error` is passed to *any other function*, then that function + takes ownership of the error. + +### Rule 1 + +> If `grpc_error` is returned by a function, the caller owns a ref to that +> instance.* + +For example, in the following code block, error1 and error2 are owned by the +current function. + +```C +grpc_error* error1 = GRPC_ERROR_CREATE("Some error occured"); +grpc_error* error2 = some_operation_that_might_fail(...); +``` + +The current function would have to explicitly call GRPC_ERROR_UNREF on the +errors, or pass them along to a function that would take over the ownership. + +### Rule 2 + +> If a `grpc_error` is passed to a `grpc_closure` callback function, then that +> function does not own a ref to the error. + +A `grpc_closure` callback function is any function that has the signature: + +```C +void (*cb)(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error); +``` + +This means that the error ownership is NOT transferred when a functions calls: + +```C +c->cb(exec_ctx, c->cb_arg, err); +``` + +The caller is still responsible for unref-ing the error. + +However, the above line is currently being phased out! It is safer to invoke +callbacks with `grpc_closure_run` and `grpc_closure_sched`. These functions are +not callbacks, so they will take ownership of the error passed to them. + +```C +grpc_error* error = GRPC_ERROR_CREATE("Some error occured"); +grpc_closure_run(exec_ctx, cb, error); +// current function no longer has ownership of the error +``` + +If you schedule or run a closure, but still need ownership of the error, then +you must explicitly take a reference. + +```C +grpc_error* error = GRPC_ERROR_CREATE("Some error occured"); +grpc_closure_run(exec_ctx, cb, GRPC_ERROR_REF(error)); +// do some other things with the error +GRPC_ERROR_UNREF(error); +``` + +Rule 2 is more important to keep in mind when **implementing** `grpc_closure` +callback functions. You must keep in mind that you do not own the error, and +must not unref it. More importantly, you cannot pass it to any function that +would take ownership of the error, without explicitly taking ownership yourself. +For example: + +```C +void on_some_action(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { + // this would cause a crash, because some_function will unref the error, + // and the caller of this callback will also unref it. + some_function(error); + + // this callback function must take ownership, so it can give that + // ownership to the function it is calling. + some_function(GRPC_ERROR_REF(error)); +} +``` + +### Rule 3 + +> if a `grpc_error` is passed to *any other function*, then that function takes +> ownership of the error. + +Take the following example: + +```C +grpc_error* error = GRPC_ERROR_CREATE("Some error occured"); +// do some things +some_function(error); +// can't use error anymore! might be gone. +``` + +When some_function is called, it takes over the ownership of the error, and it +will eventually unref it. So the caller can no longer safely use the error. + +If the caller needed to keep using the error (or passing it to other functions), +if would have to take on a reference to it. This is a common pattern seen. + +```C +void func() { + grpc_error* error = GRPC_ERROR_CREATE("Some error occured"); + some_function(GRPC_ERROR_REF(error)); + // do things + some_other_function(GRPC_ERROR_REF(error)); + // do more things + some_last_function(error); +} +``` + +The last call takes ownership and will eventually give the error its final +unref. + +When **implementing** a function that takes an error (and is not a +`grpc_closure` callback function), you must ensure the error is unref-ed either +by doing it explicitly with GRPC_ERROR_UNREF, or by passing the error to a +function that takes over the ownership. diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index eb953947ae..0e7a7b0a1e 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -45,28 +45,9 @@ extern "C" { #endif /// Opaque representation of an error. -/// Errors are refcounted objects that represent the result of an operation. -/// Ownership laws: -/// if a grpc_error is returned by a function, the caller owns a ref to that -/// instance -/// if a grpc_error is passed to a grpc_closure callback function (functions -/// with the signature: -/// void (*f)(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error)) -/// then those functions do not own a ref to error (but are free to manually -/// take a reference). -/// if a grpc_error is passed to *ANY OTHER FUNCTION* then that function takes -/// ownership of the error -/// Errors have: -/// a set of ints, strings, and timestamps that describe the error -/// always present are: -/// GRPC_ERROR_STR_FILE, GRPC_ERROR_INT_FILE_LINE - source location the error -/// was generated -/// GRPC_ERROR_STR_DESCRIPTION - a human readable description of the error -/// GRPC_ERROR_TIME_CREATED - a timestamp indicating when the error happened -/// an error can also have children; these are other errors that are believed -/// to have contributed to this one. By accumulating children, we can begin -/// to root cause high level failures from low level failures, without having -/// to derive execution paths from log lines +/// See https://github.com/grpc/grpc/blob/master/doc/core/grpc-error.md for a +/// full write up of this object. + typedef struct grpc_error grpc_error; typedef enum { diff --git a/tools/doxygen/Doxyfile.core b/tools/doxygen/Doxyfile.core index 355029ec9e..e15c157751 100644 --- a/tools/doxygen/Doxyfile.core +++ b/tools/doxygen/Doxyfile.core @@ -771,6 +771,7 @@ doc/compression_cookbook.md \ doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ doc/connectivity-semantics-and-api.md \ +doc/core/grpc-error.md \ doc/core/pending_api_cleanups.md \ doc/cpp-style-guide.md \ doc/environment_variables.md \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index db09c40dc3..dba0f4c172 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -771,6 +771,7 @@ doc/compression_cookbook.md \ doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ doc/connectivity-semantics-and-api.md \ +doc/core/grpc-error.md \ doc/core/pending_api_cleanups.md \ doc/cpp-style-guide.md \ doc/environment_variables.md \ |