diff options
-rw-r--r-- | BUILD | 1 | ||||
-rw-r--r-- | build.yaml | 1 | ||||
-rw-r--r-- | gRPC-Core.podspec | 2 | ||||
-rwxr-xr-x | grpc.gemspec | 1 | ||||
-rw-r--r-- | package.xml | 1 | ||||
-rw-r--r-- | src/core/lib/surface/alarm.c | 73 | ||||
-rw-r--r-- | src/core/lib/surface/alarm_internal.h | 40 | ||||
-rw-r--r-- | src/core/lib/surface/init.c | 2 | ||||
-rw-r--r-- | test/core/surface/alarm_test.c | 15 | ||||
-rw-r--r-- | tools/doxygen/Doxyfile.c++.internal | 1 | ||||
-rw-r--r-- | tools/doxygen/Doxyfile.core.internal | 1 | ||||
-rw-r--r-- | tools/run_tests/generated/sources_and_headers.json | 2 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc++/grpc++.vcxproj | 1 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc++/grpc++.vcxproj.filters | 3 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj | 1 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj.filters | 3 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc/grpc.vcxproj | 1 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 3 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj | 1 | ||||
-rw-r--r-- | vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters | 3 |
20 files changed, 148 insertions, 8 deletions
@@ -774,6 +774,7 @@ grpc_cc_library( "src/core/lib/slice/slice_hash_table.h", "src/core/lib/slice/slice_internal.h", "src/core/lib/slice/slice_string_helpers.h", + "src/core/lib/surface/alarm_internal.h", "src/core/lib/surface/api_trace.h", "src/core/lib/surface/call.h", "src/core/lib/surface/call_test_only.h", diff --git a/build.yaml b/build.yaml index a0a9046f65..b922c82728 100644 --- a/build.yaml +++ b/build.yaml @@ -415,6 +415,7 @@ filegroups: - src/core/lib/slice/slice_hash_table.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_string_helpers.h + - src/core/lib/surface/alarm_internal.h - src/core/lib/surface/api_trace.h - src/core/lib/surface/call.h - src/core/lib/surface/call_test_only.h diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index cbef73687b..7d5bafbc21 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -400,6 +400,7 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_hash_table.h', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_string_helpers.h', + 'src/core/lib/surface/alarm_internal.h', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/call.h', 'src/core/lib/surface/call_test_only.h', @@ -886,6 +887,7 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_hash_table.h', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_string_helpers.h', + 'src/core/lib/surface/alarm_internal.h', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/call.h', 'src/core/lib/surface/call_test_only.h', diff --git a/grpc.gemspec b/grpc.gemspec index 7618b2133a..3f8ccea870 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -332,6 +332,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/slice/slice_hash_table.h ) s.files += %w( src/core/lib/slice/slice_internal.h ) s.files += %w( src/core/lib/slice/slice_string_helpers.h ) + s.files += %w( src/core/lib/surface/alarm_internal.h ) s.files += %w( src/core/lib/surface/api_trace.h ) s.files += %w( src/core/lib/surface/call.h ) s.files += %w( src/core/lib/surface/call_test_only.h ) diff --git a/package.xml b/package.xml index 61866f3faf..5421a390fb 100644 --- a/package.xml +++ b/package.xml @@ -346,6 +346,7 @@ <file baseinstalldir="/" name="src/core/lib/slice/slice_hash_table.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/slice/slice_internal.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/slice/slice_string_helpers.h" role="src" /> + <file baseinstalldir="/" name="src/core/lib/surface/alarm_internal.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/surface/api_trace.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/surface/call.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/surface/call_test_only.h" role="src" /> diff --git a/src/core/lib/surface/alarm.c b/src/core/lib/surface/alarm.c index 55934964f3..7d60b1de17 100644 --- a/src/core/lib/surface/alarm.c +++ b/src/core/lib/surface/alarm.c @@ -15,6 +15,7 @@ * limitations under the License. * */ +#include "src/core/lib/surface/alarm_internal.h" #include <grpc/grpc.h> #include <grpc/support/alloc.h> @@ -22,7 +23,13 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/surface/completion_queue.h" +#ifndef NDEBUG +grpc_tracer_flag grpc_trace_alarm_refcount = + GRPC_TRACER_INITIALIZER(false, "alarm_refcount"); +#endif + struct grpc_alarm { + gpr_refcount refs; grpc_timer alarm; grpc_closure on_alarm; grpc_cq_completion completion; @@ -32,13 +39,58 @@ struct grpc_alarm { void *tag; }; -static void do_nothing_end_completion(grpc_exec_ctx *exec_ctx, void *arg, - grpc_cq_completion *c) {} +static void alarm_ref(grpc_alarm *alarm) { gpr_ref(&alarm->refs); } + +static void alarm_unref(grpc_alarm *alarm) { + if (gpr_unref(&alarm->refs)) { + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + GRPC_CQ_INTERNAL_UNREF(&exec_ctx, alarm->cq, "alarm"); + grpc_exec_ctx_finish(&exec_ctx); + gpr_free(alarm); + } +} + +#ifndef NDEBUG +static void alarm_ref_dbg(grpc_alarm *alarm, const char *reason, + const char *file, int line) { + if (GRPC_TRACER_ON(grpc_trace_alarm_refcount)) { + gpr_atm val = gpr_atm_no_barrier_load(&alarm->refs.count); + gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG, + "Alarm:%p ref %" PRIdPTR " -> %" PRIdPTR " %s", alarm, val, + val + 1, reason); + } + + alarm_ref(alarm); +} + +static void alarm_unref_dbg(grpc_alarm *alarm, const char *reason, + const char *file, int line) { + if (GRPC_TRACER_ON(grpc_trace_alarm_refcount)) { + gpr_atm val = gpr_atm_no_barrier_load(&alarm->refs.count); + gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG, + "Alarm:%p Unref %" PRIdPTR " -> %" PRIdPTR " %s", alarm, val, + val - 1, reason); + } + + alarm_unref(alarm); +} +#endif + +static void alarm_end_completion(grpc_exec_ctx *exec_ctx, void *arg, + grpc_cq_completion *c) { + grpc_alarm *alarm = arg; + GRPC_ALARM_UNREF(alarm, "dequeue-end-op"); +} static void alarm_cb(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { grpc_alarm *alarm = arg; - grpc_cq_end_op(exec_ctx, alarm->cq, alarm->tag, error, - do_nothing_end_completion, NULL, &alarm->completion); + + /* We are queuing an op on completion queue. This means, the alarm's structure + cannot be destroyed until the op is dequeued. Adding an extra ref + here and unref'ing when the op is dequeued will achieve this */ + GRPC_ALARM_REF(alarm, "queue-end-op"); + grpc_cq_end_op(exec_ctx, alarm->cq, alarm->tag, error, alarm_end_completion, + (void *)alarm, &alarm->completion); } grpc_alarm *grpc_alarm_create(grpc_completion_queue *cq, gpr_timespec deadline, @@ -46,6 +98,14 @@ grpc_alarm *grpc_alarm_create(grpc_completion_queue *cq, gpr_timespec deadline, grpc_alarm *alarm = gpr_malloc(sizeof(grpc_alarm)); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + gpr_ref_init(&alarm->refs, 1); + +#ifndef NDEBUG + if (GRPC_TRACER_ON(grpc_trace_alarm_refcount)) { + gpr_log(GPR_DEBUG, "Alarm:%p created (ref: 1)", alarm); + } +#endif + GRPC_CQ_INTERNAL_REF(cq, "alarm"); alarm->cq = cq; alarm->tag = tag; @@ -67,9 +127,6 @@ void grpc_alarm_cancel(grpc_alarm *alarm) { } void grpc_alarm_destroy(grpc_alarm *alarm) { - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_alarm_cancel(alarm); - GRPC_CQ_INTERNAL_UNREF(&exec_ctx, alarm->cq, "alarm"); - gpr_free(alarm); - grpc_exec_ctx_finish(&exec_ctx); + GRPC_ALARM_UNREF(alarm, "alarm_destroy"); } diff --git a/src/core/lib/surface/alarm_internal.h b/src/core/lib/surface/alarm_internal.h new file mode 100644 index 0000000000..7f2126c5c9 --- /dev/null +++ b/src/core/lib/surface/alarm_internal.h @@ -0,0 +1,40 @@ +/* + * + * Copyright 2015-2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#ifndef GRPC_CORE_LIB_SURFACE_ALARM_INTERNAL_H +#define GRPC_CORE_LIB_SURFACE_ALARM_INTERNAL_H + +#include <grpc/support/log.h> +#include "src/core/lib/debug/trace.h" + +#ifndef NDEBUG + +extern grpc_tracer_flag grpc_trace_alarm_refcount; + +#define GRPC_ALARM_REF(a, reason) alarm_ref_dbg(a, reason, __FILE__, __LINE__) +#define GRPC_ALARM_UNREF(a, reason) \ + alarm_unref_dbg(a, reason, __FILE__, __LINE__) + +#else /* !defined(NDEBUG) */ + +#define GRPC_ALARM_REF(a, reason) alarm_ref(a) +#define GRPC_ALARM_UNREF(a, reason) alarm_unref(a) + +#endif /* defined(NDEBUG) */ + +#endif /* GRPC_CORE_LIB_SURFACE_ALARM_INTERNAL_H */ diff --git a/src/core/lib/surface/init.c b/src/core/lib/surface/init.c index db111e597f..d199ac060e 100644 --- a/src/core/lib/surface/init.c +++ b/src/core/lib/surface/init.c @@ -36,6 +36,7 @@ #include "src/core/lib/iomgr/resource_quota.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/surface/alarm_internal.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/call.h" #include "src/core/lib/surface/channel_init.h" @@ -135,6 +136,7 @@ void grpc_init(void) { grpc_register_tracer(&grpc_call_error_trace); #ifndef NDEBUG grpc_register_tracer(&grpc_trace_pending_tags); + grpc_register_tracer(&grpc_trace_alarm_refcount); grpc_register_tracer(&grpc_trace_cq_refcount); grpc_register_tracer(&grpc_trace_closure); grpc_register_tracer(&grpc_trace_error_refcount); diff --git a/test/core/surface/alarm_test.c b/test/core/surface/alarm_test.c index baaa05928e..6971d92074 100644 --- a/test/core/surface/alarm_test.c +++ b/test/core/surface/alarm_test.c @@ -73,6 +73,21 @@ static void test_alarm(void) { GPR_ASSERT(ev.success == 0); grpc_alarm_destroy(alarm); } + { + /* alarm_destroy before cq_next */ + grpc_event ev; + void *tag = create_test_tag(); + grpc_alarm *alarm = + grpc_alarm_create(cc, grpc_timeout_seconds_to_deadline(2), tag); + + grpc_alarm_destroy(alarm); + ev = grpc_completion_queue_next(cc, grpc_timeout_seconds_to_deadline(1), + NULL); + GPR_ASSERT(ev.type == GRPC_OP_COMPLETE); + GPR_ASSERT(ev.tag == tag); + GPR_ASSERT(ev.success == 0); + } + shutdown_and_destroy(cc); } diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 3352431149..40392a1d33 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1033,6 +1033,7 @@ src/core/lib/support/string_windows.h \ src/core/lib/support/thd_internal.h \ src/core/lib/support/time_precise.h \ src/core/lib/support/tmpfile.h \ +src/core/lib/surface/alarm_internal.h \ src/core/lib/surface/api_trace.h \ src/core/lib/surface/call.h \ src/core/lib/surface/call_test_only.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 6a5967ad61..6a24d9a37f 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1341,6 +1341,7 @@ src/core/lib/support/tmpfile_windows.c \ src/core/lib/support/wrap_memcpy.c \ src/core/lib/surface/README.md \ src/core/lib/surface/alarm.c \ +src/core/lib/surface/alarm_internal.h \ src/core/lib/surface/api_trace.c \ src/core/lib/surface/api_trace.h \ src/core/lib/surface/byte_buffer.c \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index e6716618d8..37db0ce158 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -8035,6 +8035,7 @@ "src/core/lib/slice/slice_hash_table.h", "src/core/lib/slice/slice_internal.h", "src/core/lib/slice/slice_string_helpers.h", + "src/core/lib/surface/alarm_internal.h", "src/core/lib/surface/api_trace.h", "src/core/lib/surface/call.h", "src/core/lib/surface/call_test_only.h", @@ -8163,6 +8164,7 @@ "src/core/lib/slice/slice_hash_table.h", "src/core/lib/slice/slice_internal.h", "src/core/lib/slice/slice_string_helpers.h", + "src/core/lib/surface/alarm_internal.h", "src/core/lib/surface/api_trace.h", "src/core/lib/surface/call.h", "src/core/lib/surface/call_test_only.h", diff --git a/vsprojects/vcxproj/grpc++/grpc++.vcxproj b/vsprojects/vcxproj/grpc++/grpc++.vcxproj index 625c62adcd..04341d723a 100644 --- a/vsprojects/vcxproj/grpc++/grpc++.vcxproj +++ b/vsprojects/vcxproj/grpc++/grpc++.vcxproj @@ -506,6 +506,7 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_hash_table.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h" /> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call_test_only.h" /> diff --git a/vsprojects/vcxproj/grpc++/grpc++.vcxproj.filters b/vsprojects/vcxproj/grpc++/grpc++.vcxproj.filters index ff9913a362..a835dd73fb 100644 --- a/vsprojects/vcxproj/grpc++/grpc++.vcxproj.filters +++ b/vsprojects/vcxproj/grpc++/grpc++.vcxproj.filters @@ -869,6 +869,9 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h"> <Filter>src\core\lib\slice</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h"> + <Filter>src\core\lib\surface</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h"> <Filter>src\core\lib\surface</Filter> </ClInclude> diff --git a/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj b/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj index baeb6e35b2..0b8f322a22 100644 --- a/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj @@ -500,6 +500,7 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_hash_table.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h" /> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call_test_only.h" /> diff --git a/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj.filters index 5820ce0414..10f29fd3de 100644 --- a/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc++_unsecure/grpc++_unsecure.vcxproj.filters @@ -836,6 +836,9 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h"> <Filter>src\core\lib\slice</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h"> + <Filter>src\core\lib\surface</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h"> <Filter>src\core\lib\surface</Filter> </ClInclude> diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 96e70cc132..62c340b2c0 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -457,6 +457,7 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_hash_table.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h" /> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call_test_only.h" /> diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 925be85492..b877e3ac38 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -1328,6 +1328,9 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h"> <Filter>src\core\lib\slice</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h"> + <Filter>src\core\lib\surface</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h"> <Filter>src\core\lib\surface</Filter> </ClInclude> diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 96f87583e1..c8ff36f125 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -421,6 +421,7 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_hash_table.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h" /> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call.h" /> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\call_test_only.h" /> diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 5e26ff88cc..76dc5a66ef 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -1157,6 +1157,9 @@ <ClInclude Include="$(SolutionDir)\..\src\core\lib\slice\slice_string_helpers.h"> <Filter>src\core\lib\slice</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\alarm_internal.h"> + <Filter>src\core\lib\surface</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\src\core\lib\surface\api_trace.h"> <Filter>src\core\lib\surface</Filter> </ClInclude> |