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.

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

1.x: Workaround for CHM.keySet bad type with Java 8 compiler #5602

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

akarnokd
Copy link
Member

This PR fixes the problem that ConcurrentHashMap::keySet, when compiled on JDK 8, adds the KeySetView type in the bytecode which is not available in JDK 7 and before. The change forces the offending place to use the the standard Map::keySet which returns a standard Set.

Copy link
Member

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

👍 !

static void purgeExecutors() {
try {
Iterator<ScheduledThreadPoolExecutor> it = EXECUTORS.keySet().iterator();
Map<ScheduledThreadPoolExecutor, ScheduledThreadPoolExecutor> map = EXECUTORS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a comment I guess

@artem-zinnatullin
Copy link
Contributor

Please give me some time to run test scenario that failed before as a sanity check

Copy link
Contributor

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Checked with test scenario from artem-zinnatullin/RxJavaProGuardRules#62 (comment)

static void purgeExecutors() {
try {
Iterator<ScheduledThreadPoolExecutor> it = EXECUTORS.keySet().iterator();
// This prevents map.keySet to compile to a Java 8+ KeySetView return type
// and cause NoClassDefFound on Java 6-7 runtimes.
Copy link
Contributor

Choose a reason for hiding this comment

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

NoSuchMethodError

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@akarnokd
Copy link
Member Author

Added comment.

I assume if the PR is verified by @artem-zinnatullin, we should release 1.3.2 asap.

@artem-zinnatullin
Copy link
Contributor

mkay, looks like Travis stuck, build is in the queue for an hour.

@akarnokd can you stop/start or retrigger it?

@akarnokd
Copy link
Member Author

There is quite a backlog piled up on Travis. May not clear before I go to sleep.

@artem-zinnatullin
Copy link
Contributor

Ok, you might want to check "Concurrent jobs" related options in build settings on Travis

@akarnokd
Copy link
Member Author

It's not our single job but an overall ~400 started by other projects:

image

@codecov
Copy link

codecov bot commented Sep 14, 2017

Codecov Report

Merging #5602 into 1.x will decrease coverage by 0.13%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##                1.x    #5602      +/-   ##
============================================
- Coverage     84.34%   84.21%   -0.14%     
+ Complexity     2891     2886       -5     
============================================
  Files           291      291              
  Lines         18198    18199       +1     
  Branches       2480     2480              
============================================
- Hits          15349    15326      -23     
- Misses         1989     1999      +10     
- Partials        860      874      +14
Impacted Files Coverage Δ Complexity Δ
...n/java/rx/internal/schedulers/NewThreadWorker.java 59.43% <0%> (-0.57%) 20 <0> (ø)
...java/rx/internal/schedulers/ExecutorScheduler.java 77.46% <0%> (-7.05%) 2% <0%> (ø)
...al/schedulers/GenericScheduledExecutorService.java 70.73% <0%> (-4.88%) 12% <0%> (ø)
...ain/java/rx/internal/schedulers/SchedulerWhen.java 83.78% <0%> (-4.06%) 4% <0%> (ø)
...va/rx/internal/schedulers/EventLoopsScheduler.java 84.84% <0%> (-3.04%) 7% <0%> (ø)
src/main/java/rx/observers/SerializedObserver.java 97.82% <0%> (-2.18%) 19% <0%> (-1%)
.../rx/internal/schedulers/CachedThreadScheduler.java 89.32% <0%> (-1.95%) 6% <0%> (ø)
...x/internal/operators/DeferredScalarSubscriber.java 98.27% <0%> (-1.73%) 24% <0%> (-1%)
...a/rx/internal/operators/BufferUntilSubscriber.java 71.42% <0%> (-1.59%) 11% <0%> (-1%)
...n/java/rx/subjects/SubjectSubscriptionManager.java 80.71% <0%> (-1.43%) 15% <0%> (-1%)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed35a14...64a6e03. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants