db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mamta A. Satoor (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-3338) CancelQueryTask.forgetContext() could be simplified.
Date Mon, 09 Jun 2008 19:58:45 GMT

    [ 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.


Mime
View raw message