Set removeOnCancelPolicy on the threadpool if supported #1922
Conversation
|
@akarnokd cool, thanks for whipping this up so quickly. Since RxJava supports Java 6 we need to fix that too, right? |
|
I looked at the packages of jsr166_ but couldn't find one that ports this flag back to Java 6. |
| @@ -35,6 +34,19 @@ | |||
| /* package */ | |||
| public NewThreadWorker(ThreadFactory threadFactory) { | |||
| executor = Executors.newScheduledThreadPool(1, threadFactory); | |||
| // Java 7+: cancelled future tasks can be removed from the executor thus avoiding memory leak | |||
There was a problem hiding this comment.
Nice :-) I was wondering if we could start using reflection for things like this.
There was a problem hiding this comment.
If it is one-time only such as here, then sure.
Set removeOnCancelPolicy on the threadpool if supported
|
@benjchristensen are there plans to "fix" this for java 6 too? Since we technically support it.. or do we mark it as a limitation and people that are hit by this need to go to java 7? I'm asking because we have lots of users on java 6 and some of the requests have 75s timeouts. |
|
I'm open to a fix that works for Java 6 if someone can suggest what to do and/or submit a PR. I personally don't have the time to tackle this anytime soon but would accept the change if it solves the problem without breaking anything else (including performance) for Java 7/8. |
|
@benjchristensen okay thanks for the info. I'll tackle it if customers start to "complain", for now I also think its not as high priority - since technically jdk 6 is EOL anyway :) |
|
Yes :-) The strongest reason actually that RxJava supports Java 6 rather than 7 as the minimum is for Android support. If it becomes an issue for your customers and you want to tackle this I'll happily review, discuss and merge. |
|
You'd need to backport the Java 7 ScheduledThreadPoolExecutor and a bunch of helper classes. I looked for a backport on Doug Lea's site but no luck. |
|
How about calling |
|
That doesn't work on |
The returned |
This should solve the task retention problem of unused timeout tasks mentioned in #1919.
I know Java 7+ ScheduledThreadPoolExecutor has the
setRemoveOnCancelPolicyand I remember seeing it in Android although I can't tell from which API version.In Java 6, this can't be solved without rewriting the entire scheduler or using a backported executor.