Trace removed security providers #3473
Conversation
|
@zakkak Would something like this help with debugging provider/service removal issues? |
| } | ||
|
|
||
| private void traceRemovedProviders() { | ||
| if (removedProviders.isEmpty()) { |
There was a problem hiding this comment.
| if (removedProviders.isEmpty()) { | |
| if (removedProviders == null || removedProviders.isEmpty()) { |
There was a problem hiding this comment.
Well, it was the CI https://github.com/oracle/graal/pull/3473/checks?check_run_id=2819581300#step:9:788 not me :)
There was a problem hiding this comment.
Ah, I haven't run the gates on this internally yet, didn't check these :)
The reason a NPE could happen here - when security isn't used, analysis wont see the Providers.providerList field. This means the field value recomputer will never run and thus never clear any providers, resulting in a NPE when we access the removedProviders list
gradinac
force-pushed
the
gradinac/GR-32040-log-removed-providers
branch
from
0170a60 to
903e446
Compare
| removedProviders = new ArrayList<>(providerList.providers()); | ||
| removedProviders.removeIf(provider -> !shouldRemoveProvider(provider)); |
There was a problem hiding this comment.
Maybe it would be worth guarding this like this:
| removedProviders = new ArrayList<>(providerList.providers()); | |
| removedProviders.removeIf(provider -> !shouldRemoveProvider(provider)); | |
| if (Options.TraceSecurityServices.getValue()) { | |
| removedProviders = new ArrayList<>(providerList.providers()); | |
| removedProviders.removeIf(provider -> !shouldRemoveProvider(provider)); | |
| } |
There was a problem hiding this comment.
While I've initially done this without a guard for consistency with other tracing, I agree with you - it may be better to only compute this list only when needed to avoid overhead during non-debugging builds
gradinac
force-pushed
the
gradinac/GR-32040-log-removed-providers
branch
from
903e446 to
fe3783d
Compare
|
Thank you @gradinac ! |
3 participants
Closes #3457