Re: BUG org.postgresql.Driver.connect() distorts InterruptedException - Mailing list pgsql-jdbc

Hi,

On Thu, Aug 15, 2013 at 9:07 AM, Andreas Rudolph
<andreas.rudolph@spontech-spine.com> wrote:
> Am 13.08.2013 um 19:15 schrieb Florent Guillaume <fg@nuxeo.com>:
>> If you wrap it in a PSQLException then a JDBC connection pool could
>> take that as meaning that this connection has problems and should be
>> removed from the pool, but it won't check or propagate the interrupt,
>> and won't check the exception's cause either. Very few frameworks do.
>> That's why it's important that if you have to rewrap, you rewrap in an
>> exception that nobody risks catching without knowing that there's an
>> underlying interruption.
>
> In my bug report I was talking about another situation: The implementation of org.postgresql.Driver.connect(String
url,Properties info) does NOT leave any evidence that the calling thread (i.e. client code) has been interrupted while
waitingfor a Connection to be returned. In this situation there is no Connection at all, yet, that could be reused or
removedfrom a pool. 
> (I guess instead a pool implementation would call Driver.connect() by itself to obtain a connection for the first
time,that is the pool implementation would be a client of the Driver class.) 

A pool was just an example of a library calling the driver and needing
to do some cleanup and retry when something goes wrong. In the connect
situation one could imagine code trying to re-obtain a connection
after a few seconds if it receives an exception on the first connect.

> Even if someone agrees with your statement that receiving an InterruptedException (always) means "this thread must
die"this DOES NOT necessarily mean that it must die _immediately_! On the contrary it might need to do some clean-up,
releaseresources etc. Throwing an unchecked exception in the hope that it would not be caught is of no help in this
case.

Yes cleanup of critical resources is legitimate and encouraged on interrupt.

> The decision what an interrupt should mean to the thread in question, that is the thread that has been created by
clientcode and which executes Driver.connect(), is up to the programmer of the client code. Therefore I was asking to
improveinteroperability by allowing the client code to detect that the thread that executes it has been interrupted. 

And I'm all for it. I think it's juste better to do that in a manner
that doesn't risk being mistaken with something else by client code
(which itself could be a library in not in the hands of the
application programmer himself).

> Regarding exception handling with Driver.connect():
> Clients must catch the _checked_ SQLException. If client code has caught this exception this is a clear indication
thatno (usable) Connection object could have been returned otherwise. I can't see any evidence in your reasoning why
supplyingan InterruptedException as a cause of a checked exception would do any harm here. It just reveals the fact
thatthe connection could not be established because the requesting thread has been interrupted. 
>
> Could you please tell why doing _both_, suppling a cause _and_ setting the interrupted flag again, in the case of
Driver.connect()seems unsound to you? 

Setting the interrupt flag is definitely a must do. Setting the cause
is also a nice addition but 99% of the code out there will not check
the cause to determine that nothing should be retried and all work
should be abandoned. I'm just advocating doing what can be done in the
driver code to avoid misleading unsophisticated calling code in
thinking that processing can continue somehow.

Florent

--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com   http://www.nuxeo.org   +33 1 40 33 79 87