Skip to content

Commit

Permalink
Merge pull request grpc#17547 from markdroth/alignment_cleanup
Browse files Browse the repository at this point in the history 
Remove now-unnecessary workarounds for alignment issues.
  • Loading branch information
@markdroth
markdroth authored Dec 20, 2018
2 parents a0c7ec8 + 09a173b commit b96196f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 103 deletions.
6 changes: 0 additions & 6 deletions src/core/ext/filters/client_channel/lb_policy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,6 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
grpc_pollset_set* interested_parties_;
/// Callback to force a re-resolution.
grpc_closure* request_reresolution_;

// Dummy classes needed for alignment issues.
// See https://github.com/grpc/grpc/issues/16032 for context.
// TODO(ncteisen): remove this as soon as the issue is resolved.
channelz::ChildRefsList dummy_list_foo;
channelz::ChildRefsList dummy_list_bar;
};

} // namespace grpc_core
Expand Down
61 changes: 11 additions & 50 deletions src/core/lib/security/credentials/composite/composite_credentials.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,44 +35,6 @@

static void composite_call_metadata_cb(void* arg, grpc_error* error);

grpc_call_credentials_array::~grpc_call_credentials_array() {
for (size_t i = 0; i < num_creds_; ++i) {
creds_array_[i].~RefCountedPtr<grpc_call_credentials>();
}
if (creds_array_ != nullptr) {
gpr_free(creds_array_);
}
}

grpc_call_credentials_array::grpc_call_credentials_array(
const grpc_call_credentials_array& that)
: num_creds_(that.num_creds_) {
reserve(that.capacity_);
for (size_t i = 0; i < num_creds_; ++i) {
new (&creds_array_[i])
grpc_core::RefCountedPtr<grpc_call_credentials>(that.creds_array_[i]);
}
}

void grpc_call_credentials_array::reserve(size_t capacity) {
if (capacity_ >= capacity) {
return;
}
grpc_core::RefCountedPtr<grpc_call_credentials>* new_arr =
static_cast<grpc_core::RefCountedPtr<grpc_call_credentials>*>(gpr_malloc(
sizeof(grpc_core::RefCountedPtr<grpc_call_credentials>) * capacity));
if (creds_array_ != nullptr) {
for (size_t i = 0; i < num_creds_; ++i) {
new (&new_arr[i]) grpc_core::RefCountedPtr<grpc_call_credentials>(
std::move(creds_array_[i]));
creds_array_[i].~RefCountedPtr<grpc_call_credentials>();
}
gpr_free(creds_array_);
}
creds_array_ = new_arr;
capacity_ = capacity;
}

namespace {
struct grpc_composite_call_credentials_metadata_context {
grpc_composite_call_credentials_metadata_context(
Expand Down Expand Up @@ -103,13 +65,13 @@ static void composite_call_metadata_cb(void* arg, grpc_error* error) {
grpc_composite_call_credentials_metadata_context* ctx =
static_cast<grpc_composite_call_credentials_metadata_context*>(arg);
if (error == GRPC_ERROR_NONE) {
const grpc_call_credentials_array& inner = ctx->composite_creds->inner();
const grpc_composite_call_credentials::CallCredentialsList& inner =
ctx->composite_creds->inner();
/* See if we need to get some more metadata. */
if (ctx->creds_index < inner.size()) {
if (inner.get(ctx->creds_index++)
->get_request_metadata(
ctx->pollent, ctx->auth_md_context, ctx->md_array,
&ctx->internal_on_request_metadata, &error)) {
if (inner[ctx->creds_index++]->get_request_metadata(
ctx->pollent, ctx->auth_md_context, ctx->md_array,
&ctx->internal_on_request_metadata, &error)) {
// Synchronous response, so call ourselves recursively.
composite_call_metadata_cb(arg, error);
GRPC_ERROR_UNREF(error);
Expand All @@ -130,12 +92,11 @@ bool grpc_composite_call_credentials::get_request_metadata(
ctx = grpc_core::New<grpc_composite_call_credentials_metadata_context>(
this, pollent, auth_md_context, md_array, on_request_metadata);
bool synchronous = true;
const grpc_call_credentials_array& inner = ctx->composite_creds->inner();
const CallCredentialsList& inner = ctx->composite_creds->inner();
while (ctx->creds_index < inner.size()) {
if (inner.get(ctx->creds_index++)
->get_request_metadata(ctx->pollent, ctx->auth_md_context,
ctx->md_array,
&ctx->internal_on_request_metadata, error)) {
if (inner[ctx->creds_index++]->get_request_metadata(
ctx->pollent, ctx->auth_md_context, ctx->md_array,
&ctx->internal_on_request_metadata, error)) {
if (*error != GRPC_ERROR_NONE) break;
} else {
synchronous = false; // Async return.
Expand All @@ -149,7 +110,7 @@ bool grpc_composite_call_credentials::get_request_metadata(
void grpc_composite_call_credentials::cancel_get_request_metadata(
grpc_credentials_mdelem_array* md_array, grpc_error* error) {
for (size_t i = 0; i < inner_.size(); ++i) {
inner_.get(i)->cancel_get_request_metadata(md_array, GRPC_ERROR_REF(error));
inner_[i]->cancel_get_request_metadata(md_array, GRPC_ERROR_REF(error));
}
GRPC_ERROR_UNREF(error);
}
Expand All @@ -172,7 +133,7 @@ void grpc_composite_call_credentials::push_to_inner(
auto composite_creds =
static_cast<grpc_composite_call_credentials*>(creds.get());
for (size_t i = 0; i < composite_creds->inner().size(); ++i) {
inner_.push_back(std::move(composite_creds->inner_.get_mutable(i)));
inner_.push_back(std::move(composite_creds->inner_[i]));
}
}

Expand Down
43 changes: 7 additions & 36 deletions src/core/lib/security/credentials/composite/composite_credentials.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,43 +21,10 @@

#include <grpc/support/port_platform.h>

#include "src/core/lib/gprpp/inlined_vector.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/security/credentials/credentials.h"

// TODO(soheil): Replace this with InlinedVector once #16032 is resolved.
class grpc_call_credentials_array {
public:
grpc_call_credentials_array() = default;
grpc_call_credentials_array(const grpc_call_credentials_array& that);

~grpc_call_credentials_array();

void reserve(size_t capacity);

// Must reserve before pushing any data.
void push_back(grpc_core::RefCountedPtr<grpc_call_credentials> cred) {
GPR_DEBUG_ASSERT(capacity_ > num_creds_);
new (&creds_array_[num_creds_++])
grpc_core::RefCountedPtr<grpc_call_credentials>(std::move(cred));
}

const grpc_core::RefCountedPtr<grpc_call_credentials>& get(size_t i) const {
GPR_DEBUG_ASSERT(i < num_creds_);
return creds_array_[i];
}
grpc_core::RefCountedPtr<grpc_call_credentials>& get_mutable(size_t i) {
GPR_DEBUG_ASSERT(i < num_creds_);
return creds_array_[i];
}

size_t size() const { return num_creds_; }

private:
grpc_core::RefCountedPtr<grpc_call_credentials>* creds_array_ = nullptr;
size_t num_creds_ = 0;
size_t capacity_ = 0;
};

/* -- Composite channel credentials. -- */

class grpc_composite_channel_credentials : public grpc_channel_credentials {
Expand Down Expand Up @@ -97,6 +64,10 @@ class grpc_composite_channel_credentials : public grpc_channel_credentials {

class grpc_composite_call_credentials : public grpc_call_credentials {
public:
using CallCredentialsList =
grpc_core::InlinedVector<grpc_core::RefCountedPtr<grpc_call_credentials>,
2>;

grpc_composite_call_credentials(
grpc_core::RefCountedPtr<grpc_call_credentials> creds1,
grpc_core::RefCountedPtr<grpc_call_credentials> creds2);
Expand All @@ -111,13 +82,13 @@ class grpc_composite_call_credentials : public grpc_call_credentials {
void cancel_get_request_metadata(grpc_credentials_mdelem_array* md_array,
grpc_error* error) override;

const grpc_call_credentials_array& inner() const { return inner_; }
const CallCredentialsList& inner() const { return inner_; }

private:
void push_to_inner(grpc_core::RefCountedPtr<grpc_call_credentials> creds,
bool is_composite);

grpc_call_credentials_array inner_;
CallCredentialsList inner_;
};

#endif /* GRPC_CORE_LIB_SECURITY_CREDENTIALS_COMPOSITE_COMPOSITE_CREDENTIALS_H \
Expand Down
22 changes: 11 additions & 11 deletions test/core/security/credentials_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,14 +465,14 @@ static void test_oauth2_google_iam_composite_creds(void) {
google_iam_creds->Unref();
GPR_ASSERT(strcmp(composite_creds->type(),
GRPC_CALL_CREDENTIALS_TYPE_COMPOSITE) == 0);
const grpc_call_credentials_array& creds_array =
const grpc_composite_call_credentials::CallCredentialsList& creds_list =
static_cast<const grpc_composite_call_credentials*>(composite_creds)
->inner();
GPR_ASSERT(creds_array.size() == 2);
GPR_ASSERT(strcmp(creds_array.get(0)->type(),
GRPC_CALL_CREDENTIALS_TYPE_OAUTH2) == 0);
GPR_ASSERT(
strcmp(creds_array.get(1)->type(), GRPC_CALL_CREDENTIALS_TYPE_IAM) == 0);
GPR_ASSERT(creds_list.size() == 2);
GPR_ASSERT(strcmp(creds_list[0]->type(), GRPC_CALL_CREDENTIALS_TYPE_OAUTH2) ==
0);
GPR_ASSERT(strcmp(creds_list[1]->type(), GRPC_CALL_CREDENTIALS_TYPE_IAM) ==
0);
run_request_metadata_test(composite_creds, auth_md_ctx, state);
composite_creds->Unref();
}
Expand All @@ -492,13 +492,13 @@ class check_channel_oauth2_google_iam final : public grpc_channel_credentials {
GPR_ASSERT(call_creds != nullptr);
GPR_ASSERT(
strcmp(call_creds->type(), GRPC_CALL_CREDENTIALS_TYPE_COMPOSITE) == 0);
const grpc_call_credentials_array& creds_array =
const grpc_composite_call_credentials::CallCredentialsList& creds_list =
static_cast<const grpc_composite_call_credentials*>(call_creds.get())
->inner();
GPR_ASSERT(strcmp(creds_array.get(0)->type(),
GRPC_CALL_CREDENTIALS_TYPE_OAUTH2) == 0);
GPR_ASSERT(strcmp(creds_array.get(1)->type(),
GRPC_CALL_CREDENTIALS_TYPE_IAM) == 0);
GPR_ASSERT(
strcmp(creds_list[0]->type(), GRPC_CALL_CREDENTIALS_TYPE_OAUTH2) == 0);
GPR_ASSERT(strcmp(creds_list[1]->type(), GRPC_CALL_CREDENTIALS_TYPE_IAM) ==
0);
return nullptr;
}
};
Expand Down

0 comments on commit b96196f

Please sign in to comment.