DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #16 from Alexander Pogrebnyak 2012-03-29 16:54:55 UTC --- Dummy comment to test https://issues.apache.org/jira/browse/INFRA-4620 Please disregard. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #15 from Alexander Pogrebnyak 2012-03-29 16:52:27 UTC --- (In reply to comment #14) > r1306410 > > a day or so ago Thanks a lot for fixing this!!! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Filip Hanik changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #14 from Filip Hanik 2012-03-29 16:37:34 UTC --- r1306410 a day or so ago -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Alexander Pogrebnyak changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED | --- Comment #13 from Alexander Pogrebnyak 2012-03-29 16:32:21 UTC --- Reopening this bug, as there is no reply to my comment #9 -> https://issues.apache.org/bugzilla/show_bug.cgi?id=52066#c9. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #12 from Alexander Pogrebnyak 2012-03-29 16:30:01 UTC --- (In reply to comment #11) > http://www.apache.org/dev/#infrastructure Created bug report https://issues.apache.org/jira/browse/INFRA-4620 -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Filip Hanik changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #11 from Filip Hanik 2012-03-29 16:19:53 UTC --- http://www.apache.org/dev/#infrastructure -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #10 from Alexander Pogrebnyak 2012-03-29 14:57:47 UTC --- BTW, it is not related to this bug, but I don't know where to submit it otherwise. When I add a comment to this bug report and press `Save Changes` button Bugzilla does not redirect we to this report (52066), but instead goes to "ASF Bugzilla – Bug 48001". 48001 is completely unrelated report, but it was also submitted by me. If you could kindly reply to this comment with a proper channel I will submit this bug there. Thanks! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066
Alexander Pogrebnyak changed:
What|Removed |Added
Status|RESOLVED|REOPENED
Resolution|FIXED |
--- Comment #9 from Alexander Pogrebnyak
2012-03-29 14:52:47 UTC ---
(In reply to comment #8)
> Please note that the interrupt flag was already cleared, this commit does
> change things around a bit
> http://svn.apache.org/viewvc?rev=1306410&view=rev
> I've made an adjustment based on a suggestion on the dev list
Reviewing the code I see these lines in close() and borrowConnection
catch (InterruptedException ex) {
...
if (!getPoolProperties().getPropagateInterruptState()) {
Thread.interrupted();
}
...
}
The problem is that this still does not set the interrupted status on the
thread, that's the common behavior of many flavors of wait methods. So, in
this case the handler have to make a call to interrupt itself.
The exception handler code should do this:
catch (InterruptedException ex) {
...
if (getPoolProperties().getPropagateInterruptState()) {
Thread.currentThread().interrupt();
} else {
Thread.interrupted();
}
...
}
The fix should be applied to lines 384 and and 631 of
org.apache.tomcat.jdbc.pool.ConnectionPool in
http://svn.apache.org/viewvc?view=revision&revision=1305931
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #8 from Filip Hanik 2012-03-28 15:25:49 UTC --- Please note that the interrupt flag was already cleared, this commit does change things around a bit http://svn.apache.org/viewvc?rev=1306410&view=rev I've made an adjustment based on a suggestion on the dev list -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Filip Hanik changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED --- Comment #7 from Filip Hanik 2012-03-27 17:59:03 UTC --- Good idea Fixed in r1305931 r1305933 -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Alexander Pogrebnyak changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|WONTFIX | --- Comment #6 from Alexander Pogrebnyak 2012-03-27 17:03:49 UTC --- (In reply to comment #5) > Our library could definitely initiate the interruption. The easiest use case > would be pool.close() to get rid of all the waiting threads. and the waiting > thread in this case, would receive a SQLException > > In the above use case, it should absolutely be cleared. > > Since the waiting thread receives the SQLException, I am still not convinced. > the call DataSource.getConnection does not have any interrupted exceptions > associated with it. OK, Let's get to this problem from the other end. Is it possible to provide a user preference that by default executes a current code, but when set preserved the thread interrupted status? I really hate to implement yet another facade to your otherwise feature complete product. If this approach is acceptable to you I am willing to contribute the required patch for review. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Filip Hanik changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||WONTFIX --- Comment #5 from Filip Hanik 2012-03-27 15:16:28 UTC --- Our library could definitely initiate the interruption. The easiest use case would be pool.close() to get rid of all the waiting threads. and the waiting thread in this case, would receive a SQLException In the above use case, it should absolutely be cleared. Since the waiting thread receives the SQLException, I am still not convinced. the call DataSource.getConnection does not have any interrupted exceptions associated with it. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Alexander Pogrebnyak changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|WONTFIX | --- Comment #4 from Alexander Pogrebnyak 2012-03-20 17:34:44 UTC --- (In reply to comment #3) > The reason an interrupt here would happen is cause WE want to interrupt the > call to queue.poll, to the user, they need to be notified that their call will > fail using a SQLException. > I capitalized WE in the above quote to highlight the wrong approach to this problem. YOU, the tomcat-jdbc, would never interrupt a thread that is stuck in a `borrowConnection`. This action can only be done by the user of your library. > If we don't clear the > flag, the thread can continue to run with a interrupted status, and I'm not > sure that is a good thing. it IS a good thing, because, once again, your library should never initiate the interruption, and if the user does not handle interruption correctly it will get reminded sooner rather than later that something is wrong. Think of it as a RuntimeException. If you know how to hanlde it you will, if you don't let the framework handle it. Swallowing interrupted status is exactly the same issue that plagued Java samples in the early days, when Exception would be swallowed and nothing reported to the caller. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 Filip Hanik changed: What|Removed |Added Status|NEW |RESOLVED Resolution||WONTFIX --- Comment #3 from Filip Hanik 2012-03-20 16:32:55 UTC --- (In reply to comment #2) > (In reply to comment #1) > > > Plus, it's not OK for the general user of the library to depend on your > specific wrapper of InterruptedException into the SQLException. A user can > switch to another implementation of connection pool and that particular > library > reporting policy may not be the same as this library's. I am not sure I agree with this. The user has not invoked an "interruptable" action, the user invoked DataSource.getConnection http://docs.oracle.com/javase/6/docs/api/javax/sql/DataSource.html#getConnection() This call says nothing about interruption nor does it declare it. It is a totally different call than poll(...) http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/BlockingQueue.html#poll%28long,%20java.util.concurrent.TimeUnit%29 > > The right thing to do is to preserve the thread interruption status by calling > Thread.currentThread( ).interrupt( ), and let the client code deal with it. The reason an interrupt here would happen is cause we want to interrupt the call to queue.poll, to the user, they need to be notified that their call will fail using a SQLException. > > BTW, setting InterruptedException as a cause of SQLException is a GOOD THING, > I > don't want you to change that. We do specify the root cause for troubleshooting reasons. If we don't clear the flag, the thread can continue to run with a interrupted status, and I'm not sure that is a good thing. Feel free to reopen with a use case that may change the course of this resolution. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066
--- Comment #2 from Alexander Pogrebnyak
2011-10-26 14:31:26 UTC ---
(In reply to comment #1)
There may be quite a few clients down the call chain that require the knowledge
that interrupt is in progress, and clearing of the interrupted flag destroys
this knowledge.
It is semantically very similar to the olden days practice of swallowing
exception in the library code, e.g.
try
{
doSomethingRisky( );
}
catch ( Throwable ex )
{
}
Plus, it's not OK for the general user of the library to depend on your
specific wrapper of InterruptedException into the SQLException. A user can
switch to another implementation of connection pool and that particular library
reporting policy may not be the same as this library's.
The right thing to do is to preserve the thread interruption status by calling
Thread.currentThread( ).interrupt( ), and let the client code deal with it.
BTW, setting InterruptedException as a cause of SQLException is a GOOD THING, I
don't want you to change that.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 52066] ConnectionPool.borrowConnection swallows interrupt state.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52066 --- Comment #1 from Konstantin Kolinko 2011-10-26 14:13:51 UTC --- 1. Thread.interrupted() - clears the flag 2. new SQLException() - bails out It does sx.initCause(ex), so original InterruptedException is still there. Do you have trouble dealing with this SQLException? Whether you should follow the advise from the book depends on why the interruption was requested. I do not see any problem here. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
