Merge pull request #16324 from grpc/mehrdada-patch-1
Fix minor typo in documentation
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index 9ad2717..bc6fa0d 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
@@ -813,11 +813,7 @@
write_state_name(st), reason));
t->write_state = st;
if (st == GRPC_CHTTP2_WRITE_STATE_IDLE) {
- grpc_chttp2_stream* s;
- while (grpc_chttp2_list_pop_waiting_for_write_stream(t, &s)) {
- GRPC_CLOSURE_LIST_SCHED(&s->run_after_write);
- GRPC_CHTTP2_STREAM_UNREF(s, "chttp2:write_closure_sched");
- }
+ GRPC_CLOSURE_LIST_SCHED(&t->run_after_write);
if (t->close_transport_on_writes_finished != nullptr) {
grpc_error* err = t->close_transport_on_writes_finished;
t->close_transport_on_writes_finished = nullptr;
@@ -1212,10 +1208,7 @@
!(closure->next_data.scratch & CLOSURE_BARRIER_MAY_COVER_WRITE)) {
GRPC_CLOSURE_RUN(closure, closure->error_data.error);
} else {
- if (grpc_chttp2_list_add_waiting_for_write_stream(t, s)) {
- GRPC_CHTTP2_STREAM_REF(s, "chttp2:pending_write_closure");
- }
- grpc_closure_list_append(&s->run_after_write, closure,
+ grpc_closure_list_append(&t->run_after_write, closure,
closure->error_data.error);
}
}
@@ -2016,10 +2009,6 @@
void grpc_chttp2_cancel_stream(grpc_chttp2_transport* t, grpc_chttp2_stream* s,
grpc_error* due_to_error) {
- GRPC_CLOSURE_LIST_SCHED(&s->run_after_write);
- if (grpc_chttp2_list_remove_waiting_for_write_stream(t, s)) {
- GRPC_CHTTP2_STREAM_UNREF(s, "chttp2:pending_write_closure");
- }
if (!t->is_client && !s->sent_trailing_metadata &&
grpc_error_has_clear_grpc_status(due_to_error)) {
close_from_api(t, s, due_to_error);
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 4f1a08d..ca6e715 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -54,8 +54,6 @@
/** streams that are waiting to start because there are too many concurrent
streams on the connection */
GRPC_CHTTP2_LIST_WAITING_FOR_CONCURRENCY,
- /** streams with closures waiting to be run on a write **/
- GRPC_CHTTP2_LIST_WAITING_FOR_WRITE,
STREAM_LIST_COUNT /* must be last */
} grpc_chttp2_stream_list_id;
@@ -433,6 +431,9 @@
*/
grpc_error* close_transport_on_writes_finished;
+ /* a list of closures to run after writes are finished */
+ grpc_closure_list run_after_write;
+
/* buffer pool state */
/** have we scheduled a benign cleanup? */
bool benign_reclaimer_registered;
@@ -583,7 +584,6 @@
grpc_slice_buffer flow_controlled_buffer;
- grpc_closure_list run_after_write;
grpc_chttp2_write_cb* on_flow_controlled_cbs;
grpc_chttp2_write_cb* on_write_finished_cbs;
grpc_chttp2_write_cb* finish_after_write;
@@ -686,13 +686,6 @@
bool grpc_chttp2_list_remove_stalled_by_stream(grpc_chttp2_transport* t,
grpc_chttp2_stream* s);
-bool grpc_chttp2_list_add_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream* s);
-bool grpc_chttp2_list_pop_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream** s);
-bool grpc_chttp2_list_remove_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream* s);
-
/********* Flow Control ***************/
// Takes in a flow control action and performs all the needed operations.
diff --git a/src/core/ext/transport/chttp2/transport/stream_lists.cc b/src/core/ext/transport/chttp2/transport/stream_lists.cc
index 50bfe36..6626170 100644
--- a/src/core/ext/transport/chttp2/transport/stream_lists.cc
+++ b/src/core/ext/transport/chttp2/transport/stream_lists.cc
@@ -35,8 +35,6 @@
return "stalled_by_stream";
case GRPC_CHTTP2_LIST_WAITING_FOR_CONCURRENCY:
return "waiting_for_concurrency";
- case GRPC_CHTTP2_LIST_WAITING_FOR_WRITE:
- return "waiting_for_write";
case STREAM_LIST_COUNT:
GPR_UNREACHABLE_CODE(return "unknown");
}
@@ -216,18 +214,3 @@
grpc_chttp2_stream* s) {
return stream_list_maybe_remove(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM);
}
-
-bool grpc_chttp2_list_add_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream* s) {
- return stream_list_add(t, s, GRPC_CHTTP2_LIST_WAITING_FOR_WRITE);
-}
-
-bool grpc_chttp2_list_pop_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream** s) {
- return stream_list_pop(t, s, GRPC_CHTTP2_LIST_WAITING_FOR_WRITE);
-}
-
-bool grpc_chttp2_list_remove_waiting_for_write_stream(grpc_chttp2_transport* t,
- grpc_chttp2_stream* s) {
- return stream_list_maybe_remove(t, s, GRPC_CHTTP2_LIST_WAITING_FOR_WRITE);
-}
diff --git a/src/core/lib/security/security_connector/load_system_roots.h b/src/core/lib/security/security_connector/load_system_roots.h
index 8d4af5b..5fdec15 100644
--- a/src/core/lib/security/security_connector/load_system_roots.h
+++ b/src/core/lib/security/security_connector/load_system_roots.h
@@ -26,4 +26,4 @@
} // namespace grpc_core
-#endif /* GRPC_CORE_LIB_SECURITY_SECURITY_CONNECTOR_LOAD_SYSTEM_ROOTS_H */
\ No newline at end of file
+#endif /* GRPC_CORE_LIB_SECURITY_SECURITY_CONNECTOR_LOAD_SYSTEM_ROOTS_H */
diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs
index a6a1d8a..6ca694e 100644
--- a/src/csharp/Grpc.Core/GrpcEnvironment.cs
+++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs
@@ -50,6 +50,7 @@
static int requestCallContextPoolThreadLocalCapacity = DefaultRequestCallContextPoolThreadLocalCapacity;
static readonly HashSet<Channel> registeredChannels = new HashSet<Channel>();
static readonly HashSet<Server> registeredServers = new HashSet<Server>();
+ static readonly AtomicCounter nativeInitCounter = new AtomicCounter();
static ILogger logger = new LogLevelFilterLogger(new ConsoleLogger(), LogLevel.Off, true);
@@ -360,12 +361,25 @@
internal static void GrpcNativeInit()
{
+ if (!IsNativeShutdownAllowed && nativeInitCounter.Count > 0)
+ {
+ // Normally grpc_init and grpc_shutdown calls should come in pairs (C core does reference counting),
+ // but in case we avoid grpc_shutdown calls altogether, calling grpc_init has no effect
+ // besides incrementing an internal C core counter that could theoretically overflow.
+ // To avoid this theoretical possibility we guard repeated calls to grpc_init()
+ // with a 64-bit atomic counter (that can't realistically overflow).
+ return;
+ }
NativeMethods.Get().grpcsharp_init();
+ nativeInitCounter.Increment();
}
internal static void GrpcNativeShutdown()
{
- NativeMethods.Get().grpcsharp_shutdown();
+ if (IsNativeShutdownAllowed)
+ {
+ NativeMethods.Get().grpcsharp_shutdown();
+ }
}
/// <summary>
@@ -411,6 +425,14 @@
return GetThreadPoolSizeOrDefault();
}
+ // On some platforms (specifically iOS), thread local variables in native code
+ // require initialization/destruction. By skipping the grpc_shutdown() call,
+ // we avoid a potential crash where grpc_shutdown() has already destroyed
+ // the thread local variables, but some C core's *_destroy() methods still
+ // need to run (e.g. they may be run by finalizer thread which is out of our control)
+ // For more context, see https://github.com/grpc/grpc/issues/16294
+ private static bool IsNativeShutdownAllowed => !PlatformApis.IsXamarinIOS && !PlatformApis.IsUnityIOS;
+
private static class ShutdownHooks
{
static object staticLock = new object();
diff --git a/src/csharp/Grpc.Core/Internal/PlatformApis.cs b/src/csharp/Grpc.Core/Internal/PlatformApis.cs
index c501aa8..a8f1475 100644
--- a/src/csharp/Grpc.Core/Internal/PlatformApis.cs
+++ b/src/csharp/Grpc.Core/Internal/PlatformApis.cs
@@ -42,6 +42,7 @@
static readonly bool isMono;
static readonly bool isNetCore;
static readonly bool isUnity;
+ static readonly bool isUnityIOS;
static readonly bool isXamarin;
static readonly bool isXamarinIOS;
static readonly bool isXamarinAndroid;
@@ -63,7 +64,25 @@
isNetCore = false;
#endif
isMono = Type.GetType("Mono.Runtime") != null;
- isUnity = Type.GetType(UnityEngineApplicationClassName) != null;
+
+ // Unity
+ var unityApplicationClass = Type.GetType(UnityEngineApplicationClassName);
+ if (unityApplicationClass != null)
+ {
+ isUnity = true;
+ // Consult value of Application.platform via reflection
+ // https://docs.unity3d.com/ScriptReference/Application-platform.html
+ var platformProperty = unityApplicationClass.GetTypeInfo().GetProperty("platform");
+ var unityRuntimePlatform = platformProperty?.GetValue(null)?.ToString();
+ isUnityIOS = (unityRuntimePlatform == "IPhonePlayer");
+ }
+ else
+ {
+ isUnity = false;
+ isUnityIOS = false;
+ }
+
+ // Xamarin
isXamarinIOS = Type.GetType(XamarinIOSObjectClassName) != null;
isXamarinAndroid = Type.GetType(XamarinAndroidObjectClassName) != null;
isXamarin = isXamarinIOS || isXamarinAndroid;
@@ -98,6 +117,14 @@
}
/// <summary>
+ /// true if running on Unity iOS, false otherwise.
+ /// </summary>
+ public static bool IsUnityIOS
+ {
+ get { return isUnityIOS; }
+ }
+
+ /// <summary>
/// true if running on a Xamarin platform (either Xamarin.Android or Xamarin.iOS),
/// false otherwise.
/// </summary>
diff --git a/src/csharp/Grpc.Core/Version.csproj.include b/src/csharp/Grpc.Core/Version.csproj.include
index 6b0731e..45bd8eb 100755
--- a/src/csharp/Grpc.Core/Version.csproj.include
+++ b/src/csharp/Grpc.Core/Version.csproj.include
@@ -2,6 +2,6 @@
<Project>
<PropertyGroup>
<GrpcCsharpVersion>1.15.0-dev</GrpcCsharpVersion>
- <GoogleProtobufVersion>3.6.0</GoogleProtobufVersion>
+ <GoogleProtobufVersion>3.6.1</GoogleProtobufVersion>
</PropertyGroup>
</Project>
diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m
index 9783b06..8ce88c7 100644
--- a/src/objective-c/GRPCClient/GRPCCall.m
+++ b/src/objective-c/GRPCClient/GRPCCall.m
@@ -220,17 +220,16 @@
}
- (void)cancel {
- [self
- maybeFinishWithError:[NSError
- errorWithDomain:kGRPCErrorDomain
- code:GRPCErrorCodeCancelled
- userInfo:@{NSLocalizedDescriptionKey : @"Canceled by app"}]];
-
if (!self.isWaitingForToken) {
[self cancelCall];
} else {
self.isWaitingForToken = NO;
}
+ [self
+ maybeFinishWithError:[NSError
+ errorWithDomain:kGRPCErrorDomain
+ code:GRPCErrorCodeCancelled
+ userInfo:@{NSLocalizedDescriptionKey : @"Canceled by app"}]];
}
- (void)maybeFinishWithError:(NSError *)errorOrNil {
@@ -292,6 +291,7 @@
// don't want to throw, because the app shouldn't crash for a behavior
// that's on the hands of any server to have. Instead we finish and ask
// the server to cancel.
+ [strongSelf cancelCall];
[strongSelf
maybeFinishWithError:[NSError errorWithDomain:kGRPCErrorDomain
code:GRPCErrorCodeResourceExhausted
@@ -300,7 +300,6 @@
@"Client does not have enough memory to "
@"hold the server response."
}]];
- [strongSelf cancelCall];
return;
}
[strongWriteable enqueueValue:data
@@ -530,13 +529,17 @@
}
- (void)connectivityChanged:(NSNotification *)note {
- [self maybeFinishWithError:[NSError errorWithDomain:kGRPCErrorDomain
+ // Cancel underlying call upon this notification
+ __strong GRPCCall *strongSelf = self;
+ if (strongSelf) {
+ [self cancelCall];
+ [self
+ maybeFinishWithError:[NSError errorWithDomain:kGRPCErrorDomain
code:GRPCErrorCodeUnavailable
userInfo:@{
NSLocalizedDescriptionKey : @"Connectivity lost."
}]];
- // Cancel underlying call upon this notification
- [self cancelCall];
+ }
}
@end
diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
index f28e494..7781d27 100644
--- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
+++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
@@ -187,6 +187,7 @@
grpc_slice _details;
size_t _detailsCapacity;
grpc_metadata_array _trailers;
+ const char *_errorString;
}
- (instancetype)init {
@@ -200,6 +201,7 @@
_op.data.recv_status_on_client.status_details = &_details;
grpc_metadata_array_init(&_trailers);
_op.data.recv_status_on_client.trailing_metadata = &_trailers;
+ _op.data.recv_status_on_client.error_string = &_errorString;
if (handler) {
// Prevent reference cycle with _handler
__weak typeof(self) weakSelf = self;
@@ -207,8 +209,9 @@
__strong typeof(self) strongSelf = weakSelf;
if (strongSelf) {
char *details = grpc_slice_to_c_string(strongSelf->_details);
- NSError *error =
- [NSError grpc_errorFromStatusCode:strongSelf->_statusCode details:details];
+ NSError *error = [NSError grpc_errorFromStatusCode:strongSelf->_statusCode
+ details:details
+ errorString:strongSelf->_errorString];
NSDictionary *trailers =
[NSDictionary grpc_dictionaryFromMetadataArray:strongSelf->_trailers];
handler(error, trailers);
@@ -223,6 +226,7 @@
- (void)dealloc {
grpc_metadata_array_destroy(&_trailers);
grpc_slice_unref(_details);
+ gpr_free((void *)_errorString);
}
@end
diff --git a/src/objective-c/GRPCClient/private/NSError+GRPC.h b/src/objective-c/GRPCClient/private/NSError+GRPC.h
index e96b729..a63e76e 100644
--- a/src/objective-c/GRPCClient/private/NSError+GRPC.h
+++ b/src/objective-c/GRPCClient/private/NSError+GRPC.h
@@ -24,5 +24,7 @@
* Returns nil if the status code is OK. Otherwise, a NSError whose code is one of |GRPCErrorCode|
* and whose domain is |kGRPCErrorDomain|.
*/
-+ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode details:(char *)details;
++ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode
+ details:(char *)details
+ errorString:(const char *)errorString;
@end
diff --git a/src/objective-c/GRPCClient/private/NSError+GRPC.m b/src/objective-c/GRPCClient/private/NSError+GRPC.m
index c2e65e4..199b2eb 100644
--- a/src/objective-c/GRPCClient/private/NSError+GRPC.m
+++ b/src/objective-c/GRPCClient/private/NSError+GRPC.m
@@ -23,13 +23,19 @@
NSString *const kGRPCErrorDomain = @"io.grpc";
@implementation NSError (GRPC)
-+ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode details:(char *)details {
++ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode
+ details:(char *)details
+ errorString:(const char *)errorString {
if (statusCode == GRPC_STATUS_OK) {
return nil;
}
NSString *message = [NSString stringWithCString:details encoding:NSUTF8StringEncoding];
+ NSString *debugMessage = [NSString stringWithCString:errorString encoding:NSUTF8StringEncoding];
return [NSError errorWithDomain:kGRPCErrorDomain
code:statusCode
- userInfo:@{NSLocalizedDescriptionKey : message}];
+ userInfo:@{
+ NSLocalizedDescriptionKey : message,
+ NSDebugDescriptionErrorKey : debugMessage
+ }];
}
@end
diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m
index 2a16980..a0de8ba 100644
--- a/src/objective-c/tests/GRPCClientTests.m
+++ b/src/objective-c/tests/GRPCClientTests.m
@@ -591,4 +591,39 @@
[self testTimeoutBackoffWithTimeout:0.3 Backoff:0.7];
}
+- (void)testErrorDebugInformation {
+ __weak XCTestExpectation *expectation = [self expectationWithDescription:@"RPC unauthorized."];
+
+ RMTSimpleRequest *request = [RMTSimpleRequest message];
+ request.fillUsername = YES;
+ request.fillOauthScope = YES;
+ GRXWriter *requestsWriter = [GRXWriter writerWithValue:[request data]];
+
+ GRPCCall *call = [[GRPCCall alloc] initWithHost:kRemoteSSLHost
+ path:kUnaryCallMethod.HTTPPath
+ requestsWriter:requestsWriter];
+
+ call.oauth2AccessToken = @"bogusToken";
+
+ id<GRXWriteable> responsesWriteable =
+ [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) {
+ XCTFail(@"Received unexpected response: %@", value);
+ }
+ completionHandler:^(NSError *errorOrNil) {
+ XCTAssertNotNil(errorOrNil, @"Finished without error!");
+ NSDictionary *userInfo = errorOrNil.userInfo;
+ NSString *debugInformation = userInfo[NSDebugDescriptionErrorKey];
+ XCTAssertNotNil(debugInformation);
+ XCTAssertNotEqual([debugInformation length], 0);
+ NSString *challengeHeader = call.oauth2ChallengeHeader;
+ XCTAssertGreaterThan(challengeHeader.length, 0, @"No challenge in response headers %@",
+ call.responseHeaders);
+ [expectation fulfill];
+ }];
+
+ [call startWithWriteable:responsesWriteable];
+
+ [self waitForExpectationsWithTimeout:TEST_TIMEOUT handler:nil];
+}
+
@end
diff --git a/templates/src/csharp/Grpc.Core/Version.csproj.include.template b/templates/src/csharp/Grpc.Core/Version.csproj.include.template
index a950b16..0ec0a08 100755
--- a/templates/src/csharp/Grpc.Core/Version.csproj.include.template
+++ b/templates/src/csharp/Grpc.Core/Version.csproj.include.template
@@ -4,6 +4,6 @@
<Project>
<PropertyGroup>
<GrpcCsharpVersion>${settings.csharp_version}</GrpcCsharpVersion>
- <GoogleProtobufVersion>3.6.0</GoogleProtobufVersion>
+ <GoogleProtobufVersion>3.6.1</GoogleProtobufVersion>
</PropertyGroup>
</Project>
diff --git a/tools/run_tests/artifacts/artifact_targets.py b/tools/run_tests/artifacts/artifact_targets.py
index a37098d..bdeb258 100644
--- a/tools/run_tests/artifacts/artifact_targets.py
+++ b/tools/run_tests/artifacts/artifact_targets.py
@@ -290,15 +290,9 @@
return []
def build_jobspec(self):
- if self.platform == 'linux':
- return create_docker_jobspec(
- self.name, 'tools/dockerfile/grpc_artifact_linux_{}'.format(
- self.arch),
- 'tools/run_tests/artifacts/build_artifact_php.sh')
- else:
- return create_jobspec(
- self.name, ['tools/run_tests/artifacts/build_artifact_php.sh'],
- use_workspace=True)
+ return create_docker_jobspec(
+ self.name, 'tools/dockerfile/grpc_artifact_linux_{}'.format(
+ self.arch), 'tools/run_tests/artifacts/build_artifact_php.sh')
class ProtocArtifact:
@@ -400,6 +394,5 @@
PythonArtifact('windows', 'x64', 'Python37'),
RubyArtifact('linux', 'x64'),
RubyArtifact('macos', 'x64'),
- PHPArtifact('linux', 'x64'),
- PHPArtifact('macos', 'x64')
+ PHPArtifact('linux', 'x64')
])