[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change plugin credentials API to support both sync and async modes #12374

Merged
merged 16 commits into from
Sep 29, 2017
Merged

Change plugin credentials API to support both sync and async modes #12374

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2caf021
Change plugin credentials API to support both sync and async modes.
markdroth Sep 1, 2017
d27504f
Changes
kpayson64 Sep 1, 2017
6d8cc7c
Fixed some PHP errors
stanley-cheung Sep 5, 2017
8cc7e67
Merge pull request #4 from stanley-cheung/plugin_credentials_api_fix
markdroth Sep 5, 2017
27670c8
make a guarantee that c# auth callbacks are async to c-core
apolcyn Sep 6, 2017
2c4e7d7
Merge pull request #5 from apolcyn/guarantee_csharp_creds_async
markdroth Sep 6, 2017
1ff6ee1
clang-format
markdroth Sep 6, 2017
4fe48e6
Build fixes
kpayson64 Sep 7, 2017
c7388e5
Fix node build.
markdroth Sep 8, 2017
0aee498
Merge remote-tracking branch 'upstream/master' into plugin_credential…
markdroth Sep 8, 2017
09fe5de
Merge remote-tracking branch 'upstream/master' into plugin_credential…
markdroth Sep 11, 2017
ad9208c
clang-format
markdroth Sep 11, 2017
b4c01f9
Merge remote-tracking branch 'upstream/master' into plugin_credential…
markdroth Sep 25, 2017
1a8bb82
Add tracer for plugin credentials.
markdroth Sep 26, 2017
61b26f9
Code review changes.
markdroth Sep 28, 2017
6456e49
Merge remote-tracking branch 'upstream/master' into plugin_credential…
markdroth Sep 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Code review changes.
  • Loading branch information
@markdroth
markdroth committed Sep 28, 2017
commit 61b26f97b04b4fec82d6b9ea110ad7743a0ab178
2 changes: 1 addition & 1 deletion include/grpc/grpc_security.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ typedef struct {
void *reserved;
} grpc_auth_metadata_context;

/** Maximum number of credentials returnable by a credentials plugin via
/** Maximum number of metadata entries returnable by a credentials plugin via
a synchronous return. */
#define GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX 4
Copy link
Contributor
@fengli79 fengli79 Sep 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it configurable? Php need to get the callback always invoked on the same thread. Thus Php may use a larger threshold to make sure it's always sync.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we can't make it configurable. For performance reasons, we need to allocate space for the return parameters on the stack instead of heap-allocating it, so the number has to be set at compile time.

This limitation may go away in the future with some metadata API changes that @ctiller has in mind. But for now, if PHP needs to have a plugin return more then 4 metadata elements, then it needs to use the async API.


Expand Down
4 changes: 2 additions & 2 deletions src/cpp/client/secure_credentials.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,12 @@ int MetadataCredentialsPluginWrapper::GetMetadata(
w->thread_pool_->Add(
std::bind(&MetadataCredentialsPluginWrapper::InvokePlugin, w, context,
cb, user_data, nullptr, nullptr, nullptr, nullptr));
return false;
return 0;
} else {
// Synchronous return.
w->InvokePlugin(context, cb, user_data, creds_md, num_creds_md, status,
error_details);
return true;
return 1;
}
}

Expand Down