You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Alex Burgel (JIRA)" <ji...@apache.org> on 2007/04/24 15:03:34 UTC
[jira] Created: (AMQ-1235) Scheduler.cancel uses incorrect argument
to shutdown threads
Scheduler.cancel uses incorrect argument to shutdown threads
------------------------------------------------------------
Key: AMQ-1235
URL: https://issues.apache.org/activemq/browse/AMQ-1235
Project: ActiveMQ
Issue Type: Bug
Affects Versions: 4.1.1
Reporter: Alex Burgel
Priority: Critical
looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AMQ-1235) Scheduler.cancel uses incorrect argument
to shutdown threads
Posted by "Alex Burgel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1235?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Burgel updated AMQ-1235:
-----------------------------
Attachment: scheduler.patch
here's a patch.
> Scheduler.cancel uses incorrect argument to shutdown threads
> ------------------------------------------------------------
>
> Key: AMQ-1235
> URL: https://issues.apache.org/activemq/browse/AMQ-1235
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.1
> Reporter: Alex Burgel
> Priority: Critical
> Attachments: scheduler.patch
>
>
> looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
> the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
> the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
> i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
> this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AMQ-1235) Scheduler.cancel uses incorrect
argument to shutdown threads
Posted by "Alex Burgel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1235?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39041 ]
Alex Burgel commented on AMQ-1235:
----------------------------------
a quick clarification....
you can't call ScheduledThreadPoolExecutor.remove with a ScheduledFuture, because it expects a Runnable.
two alternatives are
1) don't call ScheduledThreadPoolExecutor.remove at all... which is equivalent to the current behavior, tho this wil leave the executor with lots of cancelled tasks (see AMQ-1205)
2) cast ticket to RunnableScheduledFuture, and call remove on that. this will work because in this case ticket is a RunnableScheduledFuture, tho its probably worthwhile adding an instanceof check just to be sure.
> Scheduler.cancel uses incorrect argument to shutdown threads
> ------------------------------------------------------------
>
> Key: AMQ-1235
> URL: https://issues.apache.org/activemq/browse/AMQ-1235
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.1
> Reporter: Alex Burgel
> Priority: Critical
>
> looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
> the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
> the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
> i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
> this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (AMQ-1235) Scheduler.cancel uses incorrect
argument to shutdown threads
Posted by "Hiram Chirino (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1235?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Hiram Chirino resolved AMQ-1235.
--------------------------------
Resolution: Fixed
Fix Version/s: 4.2.0
4.1.2
Good patch! Thanks for figuring this out. Applied.
> Scheduler.cancel uses incorrect argument to shutdown threads
> ------------------------------------------------------------
>
> Key: AMQ-1235
> URL: https://issues.apache.org/activemq/browse/AMQ-1235
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.1
> Reporter: Alex Burgel
> Assigned To: Hiram Chirino
> Priority: Critical
> Fix For: 4.1.2, 4.2.0
>
> Attachments: scheduler.patch
>
>
> looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
> the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
> the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
> i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
> this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (AMQ-1235) Scheduler.cancel uses incorrect
argument to shutdown threads
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1235?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
james strachan reassigned AMQ-1235:
-----------------------------------
Assignee: Hiram Chirino
> Scheduler.cancel uses incorrect argument to shutdown threads
> ------------------------------------------------------------
>
> Key: AMQ-1235
> URL: https://issues.apache.org/activemq/browse/AMQ-1235
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.1
> Reporter: Alex Burgel
> Assigned To: Hiram Chirino
> Priority: Critical
> Attachments: scheduler.patch
>
>
> looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
> the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
> the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
> i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
> this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AMQ-1235) Scheduler.cancel uses incorrect
argument to shutdown threads
Posted by "james strachan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/activemq/browse/AMQ-1235?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_39082 ]
james strachan commented on AMQ-1235:
-------------------------------------
I made an amendment to the patch to check for instanceof Runnable rather than RunnableScheduledFuture which is a Java 6 specific class
> Scheduler.cancel uses incorrect argument to shutdown threads
> ------------------------------------------------------------
>
> Key: AMQ-1235
> URL: https://issues.apache.org/activemq/browse/AMQ-1235
> Project: ActiveMQ
> Issue Type: Bug
> Affects Versions: 4.1.1
> Reporter: Alex Burgel
> Assigned To: Hiram Chirino
> Priority: Critical
> Fix For: 4.1.2, 4.2.0
>
> Attachments: scheduler.patch
>
>
> looking at the code from 4.1.1 in org.apache.activemq.thread.Scheduler, in the cancel method:
> the Runnable task argument is passed to clockDaemon.remove(). i think this is incorrect. ScheduledFuture ticket should be passed to clockDaemon.remove().
> the javadocs of ScheduledThreadPoolExecutor.remove discuss the possibility that Runnables might be stored in some other form internally, so calling remove with a plain Runnable might not do anything. I think the solution is to call remove with a ScheduledFuture, which is how they are stored internally in ScheduledThreadPoolExecutor.
> i came across this bug after upgrading to the java 5 version of backport-util-concurrent 3.0. that version makes more assumptions about the types that are passed into ScheduledThreadPoolExecutor.remove, so when you pass in a regular Runnable you'll get a ClassCastException.
> this is trivial to fix, so i don't think a patch is necessary. also i think this might address the memory leak mentioned in AMQ-1205
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.