-
Notifications
You must be signed in to change notification settings - Fork 199
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.
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
Should add null check when put data into concurrenthashmap #526
Comments
@jdneo thanks for pointing this out. By the way recent master also might have the same issue. Is it possible for you to send out a PR for this. Your contribution would be greatly appreciated. Let me know. |
Hi @dhaval24 Sure, I'll send a PR sooner. |
@jdneo thank you very much. Just curios have you encountered this issue while SDK is running? Wondering how you caught this up. |
Yes, some of our users encounter this problem. Here are some related links: |
I see. So just was checking the java docs and found that standard maps allow null keys and values but concurrent map doesn't allow them. Since this method is generic for any kind of maps it would make sense to apply check with condition on type too (source instance of ConcurrentHashMap && entry.getValue() != null) |
Yes, but if entry.getValue() is null, what should we do? Since we have no ide what type is for |
In case of concurrent maps, key with null value doesn't make sense at all so we might skip the key itself. and then log a trace with the help of Internal SDK Logger. (InternalLogger.instance.trace("Skipping key %s while copying because of null value", value) |
@dhaval24 That's fine! |
@jdneo please also do add a test case for it :-) |
The code here:
https://github.com/Microsoft/ApplicationInsights-Java/blob/v1.0.9/core/src/main/java/com/microsoft/applicationinsights/internal/util/MapUtil.java#L43
put data into a ConCurrentHashMap but does not have null check.
which will casue an NPE error if the
entry.getValue()
is null.The text was updated successfully, but these errors were encountered: