Skip to content

Commit

Permalink
[1/3] Bump grpc to 1.34.1 to fix corruption when downloading CAS blobs
Browse files Browse the repository at this point in the history
Part 1: add v1.34.1 version to third_party/grpc
Note: partly switches to v1.34.1 too as not all bits are versioned and
      some of unversioned bits are used from other third_party targets

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

bazelbuild#12927
  • Loading branch information
scele committed Feb 24, 2021
1 parent 4fe0f87 commit 8311cff
Show file tree
Hide file tree
Showing 14 changed files with 171 additions and 57 deletions.
18 changes: 9 additions & 9 deletions third_party/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ load("//tools/distributions:distribution_rules.bzl", "distrib_java_import", "dis

licenses(["notice"]) # Apache v2

exports_files(["grpc_1.32.0.patch"])
exports_files(["grpc_1.32.0.patch", "grpc_1.34.1.patch"])

package(default_visibility = ["//visibility:public"])

Expand All @@ -32,14 +32,14 @@ filegroup(
distrib_jar_filegroup(
name = "bootstrap-grpc-jars",
srcs = [
"grpc-api-1.32.2.jar",
"grpc-auth-1.32.2.jar",
"grpc-context-1.32.2.jar",
"grpc-core-1.32.2.jar",
"grpc-netty-1.32.2.jar",
"grpc-protobuf-1.32.2.jar",
"grpc-protobuf-lite-1.32.2.jar",
"grpc-stub-1.32.2.jar",
"grpc-api-1.34.1.jar",
"grpc-auth-1.34.1.jar",
"grpc-context-1.34.1.jar",
"grpc-core-1.34.1.jar",
"grpc-netty-1.34.1.jar",
"grpc-protobuf-1.34.1.jar",
"grpc-protobuf-lite-1.34.1.jar",
"grpc-stub-1.34.1.jar",
],
enable_distributions = ["debian"],
)
Expand Down
12 changes: 6 additions & 6 deletions third_party/grpc/README.bazel.md
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
# How to update the C++ sources of gRPC:

1. Update the gRPC definitions in WORKSPACE file, currently we use
https://github.com/grpc/grpc/archive/v1.32.2.tar.gz
https://github.com/grpc/grpc/archive/v1.34.1.tar.gz
2. Update the gRPC patch file if necessary, it mostly helps avoid unnecessary dependencies.
3. Update third_party/grpc/BUILD to redirect targets to @com_github_grpc_grpc if necessary.

# How to update the BUILD/bzl sources of gRPC:

1. `git clone http://github.com/grpc/grpc.git` in a convenient directory
2. `git checkout <tag>` (current is `v1.32.0`, commithash `414bb8322d`)
2. `git checkout <tag>` (current is `v1.34.1`, commithash `e5276ec369`)
3. `mkdir -p third_party/grpc/bazel`
4. `cp <gRPC git tree>/bazel/{BUILD,cc_grpc_library.bzl,generate_cc.bzl,protobuf.bzl} third_party/grpc/bazel`
5. In the `third_party/grpc` directory, apply local patches:
`patch -p3 < bazel_1.32.0.patch`
`patch -p3 < bazel_1.34.1.patch`

# How to update the Java plugin:

1. Checkout tag `v1.32.2` from https://github.com/grpc/grpc-java
1. Checkout tag `v1.34.1` from https://github.com/grpc/grpc-java
2. `cp -R <grpc-java git tree>/compiler/src/java_plugin third_party/grpc/compiler/src`

# How to update the Java code:

Download the necessary jars at version `1.32.2` from maven central.
Download the necessary jars at version `1.34.1` from maven central.

# Submitting the change needs 3 pull requests

1. Update third_party/grpc to include files from new version
2. Switch WORKSPACE, scripts/bootstrap/compile.sh and any other references to new version
2. Switch distdir_deps.bzl, scripts/bootstrap/compile.sh and any other references to new version
3. Remove older version from third_party/grpc
File renamed without changes.
56 changes: 16 additions & 40 deletions third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -713,11 +713,13 @@ static void PrintStub(
if (client_streaming) {
p->Print(
*vars,
"return asyncUnimplementedStreamingCall($method_method_name$(), responseObserver);\n");
"return io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall("
"$method_method_name$(), responseObserver);\n");
} else {
p->Print(
*vars,
"asyncUnimplementedUnaryCall($method_method_name$(), responseObserver);\n");
"io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall("
"$method_method_name$(), responseObserver);\n");
}
break;
default:
Expand All @@ -729,10 +731,10 @@ static void PrintStub(
GRPC_CODEGEN_CHECK(!client_streaming)
<< "Blocking client streaming interface is not available";
if (server_streaming) {
(*vars)["calls_method"] = "blockingServerStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.blockingServerStreamingCall";
(*vars)["params"] = "request";
} else {
(*vars)["calls_method"] = "blockingUnaryCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.blockingUnaryCall";
(*vars)["params"] = "request";
}
p->Print(
Expand All @@ -743,18 +745,18 @@ static void PrintStub(
case ASYNC_CALL:
if (server_streaming) {
if (client_streaming) {
(*vars)["calls_method"] = "asyncBidiStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.asyncBidiStreamingCall";
(*vars)["params"] = "responseObserver";
} else {
(*vars)["calls_method"] = "asyncServerStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.asyncServerStreamingCall";
(*vars)["params"] = "request, responseObserver";
}
} else {
if (client_streaming) {
(*vars)["calls_method"] = "asyncClientStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.asyncClientStreamingCall";
(*vars)["params"] = "responseObserver";
} else {
(*vars)["calls_method"] = "asyncUnaryCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.asyncUnaryCall";
(*vars)["params"] = "request, responseObserver";
}
}
Expand All @@ -769,7 +771,7 @@ static void PrintStub(
<< "Future interface doesn't support streaming. "
<< "client_streaming=" << client_streaming << ", "
<< "server_streaming=" << server_streaming;
(*vars)["calls_method"] = "futureUnaryCall";
(*vars)["calls_method"] = "io.grpc.stub.ClientCalls.futureUnaryCall";
p->Print(
*vars,
"return $calls_method$(\n"
Expand Down Expand Up @@ -1031,15 +1033,15 @@ static void PrintBindServiceMethodBody(const ServiceDescriptor* service,
bool server_streaming = method->server_streaming();
if (client_streaming) {
if (server_streaming) {
(*vars)["calls_method"] = "asyncBidiStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ServerCalls.asyncBidiStreamingCall";
} else {
(*vars)["calls_method"] = "asyncClientStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ServerCalls.asyncClientStreamingCall";
}
} else {
if (server_streaming) {
(*vars)["calls_method"] = "asyncServerStreamingCall";
(*vars)["calls_method"] = "io.grpc.stub.ServerCalls.asyncServerStreamingCall";
} else {
(*vars)["calls_method"] = "asyncUnaryCall";
(*vars)["calls_method"] = "io.grpc.stub.ServerCalls.asyncUnaryCall";
}
}
p->Print(*vars, ".addMethod(\n");
Expand Down Expand Up @@ -1160,33 +1162,7 @@ static void PrintService(const ServiceDescriptor* service,
void PrintImports(Printer* p) {
p->Print(
"import static "
"io.grpc.MethodDescriptor.generateFullMethodName;\n"
"import static "
"io.grpc.stub.ClientCalls.asyncBidiStreamingCall;\n"
"import static "
"io.grpc.stub.ClientCalls.asyncClientStreamingCall;\n"
"import static "
"io.grpc.stub.ClientCalls.asyncServerStreamingCall;\n"
"import static "
"io.grpc.stub.ClientCalls.asyncUnaryCall;\n"
"import static "
"io.grpc.stub.ClientCalls.blockingServerStreamingCall;\n"
"import static "
"io.grpc.stub.ClientCalls.blockingUnaryCall;\n"
"import static "
"io.grpc.stub.ClientCalls.futureUnaryCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncBidiStreamingCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncClientStreamingCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncServerStreamingCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncUnaryCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncUnimplementedStreamingCall;\n"
"import static "
"io.grpc.stub.ServerCalls.asyncUnimplementedUnaryCall;\n\n");
"io.grpc.MethodDescriptor.generateFullMethodName;\n\n");
}

void GenerateService(const ServiceDescriptor* service,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ class LogHelper {
}
};

// Abort the program after logging the mesage if the given condition is not
// Abort the program after logging the message if the given condition is not
// true. Otherwise, do nothing.
#define GRPC_CODEGEN_CHECK(x) !(x) && LogHelper(&std::cerr).get_os() \
<< "CHECK FAILED: " << __FILE__ << ":" \
<< __LINE__ << ": "

// Abort the program after logging the mesage.
// Abort the program after logging the message.
#define GRPC_CODEGEN_FAIL GRPC_CODEGEN_CHECK(false)

namespace java_grpc_generator {
Expand Down
Binary file added third_party/grpc/grpc-api-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-auth-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-context-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-core-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-netty-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-lite-1.34.1.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-stub-1.34.1.jar
Binary file not shown.
138 changes: 138 additions & 0 deletions third_party/grpc/grpc_1.34.1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
commit bb0d04663c7dc6c0096f8717cb4ec26330a5ae40
Author: Yun Peng <pcloudy@google.com>
Date: Wed Jun 3 15:35:31 2020 +0200

Patch grpc v1.26.0 for Bazel build

- Avoid loading dependencies that're not needed for the gRPC C++
libraries
- Add bazel mirror URL for upb and cares
- Redirect zlib to @//third_party/zlib
- Add darwin_arm64 and darwin_arm64e config settings to allow
building for Apple Silicon

diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl
index 7bb6b8bdb9..7644107b70 100644
--- a/bazel/grpc_build_system.bzl
+++ b/bazel/grpc_build_system.bzl
@@ -25,7 +25,7 @@

load("//bazel:cc_grpc_library.bzl", "cc_grpc_library")
load("@upb//bazel:upb_proto_library.bzl", "upb_proto_library")
-load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")
+# load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")

# The set of pollers to test against if a test exercises polling
POLLERS = ["epollex", "epoll1", "poll"]
@@ -181,13 +181,13 @@ def ios_cc_test(
testonly = 1,
)
ios_test_deps = [ios_test_adapter, ":" + test_lib_ios]
- ios_unit_test(
- name = name + "_on_ios",
- size = kwargs.get("size"),
- tags = ios_tags,
- minimum_os_version = "9.0",
- deps = ios_test_deps,
- )
+ # ios_unit_test(
+ # name = name + "_on_ios",
+ # size = kwargs.get("size"),
+ # tags = ios_tags,
+ # minimum_os_version = "9.0",
+ # deps = ios_test_deps,
+ # )

def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], uses_polling = True, language = "C++", size = "medium", timeout = None, tags = [], exec_compatible_with = [], exec_properties = {}, shard_count = None, flaky = None):
copts = if_mac(["-DGRPC_CFSTREAM"])
diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl
index 09fcad95a2..9b737e5deb 100644
--- a/bazel/grpc_deps.bzl
+++ b/bazel/grpc_deps.bzl
@@ -33,7 +33,7 @@ def grpc_deps():

native.bind(
name = "madler_zlib",
- actual = "@zlib//:zlib",
+ actual = "@//third_party/zlib",
)

native.bind(
@@ -288,11 +288,11 @@ def grpc_deps():
if "upb" not in native.existing_rules():
http_archive(
name = "upb",
- sha256 = "7992217989f3156f8109931c1fc6db3434b7414957cb82371552377beaeb9d6c",
- strip_prefix = "upb-382d5afc60e05470c23e8de19b19fc5ad231e732",
+ sha256 = "c0b97bf91dfea7e8d7579c24e2ecdd02d10b00f3c5defc3dce23d95100d0e664",
+ strip_prefix = "upb-60607da72e89ba0c84c84054d2e562d8b6b61177",
urls = [
- "https://storage.googleapis.com/grpc-bazel-mirror/github.com/protocolbuffers/upb/archive/382d5afc60e05470c23e8de19b19fc5ad231e732.tar.gz",
- "https://github.com/protocolbuffers/upb/archive/382d5afc60e05470c23e8de19b19fc5ad231e732.tar.gz",
+ "https://storage.googleapis.com/grpc-bazel-mirror/github.com/protocolbuffers/upb/archive/60607da72e89ba0c84c84054d2e562d8b6b61177.tar.gz",
+ "https://github.com/protocolbuffers/upb/archive/60607da72e89ba0c84c84054d2e562d8b6b61177.tar.gz",
],
)

diff --git a/bazel/grpc_extra_deps.bzl b/bazel/grpc_extra_deps.bzl
index 4c1dfad2e8..f63c54ddef 100644
--- a/bazel/grpc_extra_deps.bzl
+++ b/bazel/grpc_extra_deps.bzl
@@ -1,11 +1,6 @@
"""Loads the dependencies necessary for the external repositories defined in grpc_deps.bzl."""

-load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
load("@upb//bazel:workspace_deps.bzl", "upb_deps")
-load("@envoy_api//bazel:repositories.bzl", "api_dependencies")
-load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
-load("@build_bazel_rules_apple//apple:repositories.bzl", "apple_rules_dependencies")
-load("@build_bazel_apple_support//lib:repositories.bzl", "apple_support_dependencies")

def grpc_extra_deps():
"""Loads the extra dependencies.
@@ -26,15 +21,5 @@ def grpc_extra_deps():
grpc_extra_deps()
```
"""
- protobuf_deps()
-
upb_deps()

- api_dependencies()
-
- go_rules_dependencies()
- go_register_toolchains()
-
- apple_rules_dependencies()
-
- apple_support_dependencies()
diff --git a/third_party/cares/cares.BUILD b/third_party/cares/cares.BUILD
index c047f0c515..7c24fbc617 100644
--- a/third_party/cares/cares.BUILD
+++ b/third_party/cares/cares.BUILD
@@ -10,6 +10,16 @@ config_setting(
values = {"cpu": "darwin_x86_64"},
)

+config_setting(
+ name = "darwin_arm64",
+ values = {"cpu": "darwin_arm64"},
+)
+
+config_setting(
+ name = "darwin_arm64e",
+ values = {"cpu": "darwin_arm64e"},
+)
+
config_setting(
name = "windows",
values = {"cpu": "x64_windows"},
@@ -99,6 +109,8 @@ copy_file(
":watchos_arm64_32": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":darwin": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":darwin_x86_64": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
+ ":darwin_arm64": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
+ ":darwin_arm64e": "@com_github_grpc_grpc//third_party/cares:config_darwin/ares_config.h",
":windows": "@com_github_grpc_grpc//third_party/cares:config_windows/ares_config.h",
":android": "@com_github_grpc_grpc//third_party/cares:config_android/ares_config.h",
"//conditions:default": "@com_github_grpc_grpc//third_party/cares:config_linux/ares_config.h",

0 comments on commit 8311cff

Please sign in to comment.