[
https://issues.apache.org/jira/browse/DERBY-3338?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12603667#action_12603667
]
Mamta A. Satoor commented on DERBY-3338:
----------------------------------------
Reworking the code as suggested by Dan may actually fix the problem experienced
on weme6.1 (DERBY-1848 for jdbcapi/SetQueryTimeoutTest.java) The way the
CancelQueryTask.forgetContext() is written right now, it leaves a small window
of code path through it that leaves the CancelTimerTask linked to the
StatementContext, a timing window allows the CancelTimerTask's run method to
later cancel a different statement once the StatementContext is re-used. A
short example here might be helpful.
Let Stmt1A be a JDBC Statement object(with set query timeout set on it) and SC1
be a StatementContext. Say that user has requested a ResultSet object movement
for Stmt1A. At the start of the ResultSet movement code, we use a
StatementContext (say in this case it is SC1) and push that StatementContext
for Stmt1A. During the push, we start a Timer say CQt1 since user has set query
timeout for Stmt1A. After the ResultSet movement code, Stmt1A starts its pop on
SC1. During the pop, say the Timer CQt1 expires and so CQT1 is queued upto run.
Before CQT1 runs, say the control goes back to finish the rest of the pop SC1
code. During the pop, we call CancelQueryTask.forgetContext(). The current code
here checks if the Timer has been cancelled. If not cancelled, then we mark the
StatementContext associated with CQT1 as null otherwise we don't touch
StatementContext object associated with CQT1. Since in our specific case, the
Timer has already been cancelled, we leave SC1 associated with CQT1. After this
we finish the rest of the code to pop SC1 for Stmt1A. Keep in mind that CQT1
has not run yet. Since SC1 is available for some other Statement to use, say
Stmt2B pushes SC1 before it does it's ResultSet movement. So, now SC1 is
associated with Stmt2B. At this point, say CQT1's run gets scheduled. Since it
still has SC1 associated with it (because we never cleared it in
forgetContext), CQT1 is going to mark SC1 as timedout. This is where we run
into problem. We are indirectly marking wrong JDBC Statement as timed out (in
this particular case Stmt2B) and that is what I think is causing occassional
timeouts in jdbcapi/SetQueryTimeoutTest.java. I have rearranged the code and
run the tests and I have not been able to reproduce the problem with
jdbcapi/SetQueryTimeoutTest.java even after 30 runs (normally it reproduces for
me in about 3-5 runs). I will attach the simple patch for review and will plan
on committing it tomorrow if no one has any comments.
> CancelQueryTask.forgetContext() could be simplified.
> ----------------------------------------------------
>
> Key: DERBY-3338
> URL: https://issues.apache.org/jira/browse/DERBY-3338
> Project: Derby
> Issue Type: Improvement
> Components: Newcomer, Services, SQL
> Reporter: Daniel John Debrunner
> Assignee: Mamta A. Satoor
> Priority: Minor
>
> Minor issue but CancelQueryTask.forgetContext() has this code (in
> GenericStatementContext.java)
> public void forgetContext() {
> boolean mayStillRun = !cancel();
> if (mayStillRun) {
> synchronized (this) {
> statementContext = null;
> }
> }
> }
> The mayStillRun = !cancel() is somewhat confusing. I can't see from the
> javadoc of TimerTask.cancel() how its return value could indicate the task
> may still run.
> Less confusing code could be:
> public void forgetContext() {
> synchronized (this) {
> statementContext = null;
> }
> cancel();
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
