[
https://issues.apache.org/jira/browse/DERBY-4279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877328#action_12877328
]
Brett Wooldridge commented on DERBY-4279:
-----------------------------------------
This defect is a major issue for my company and product. I am submitting a
patch to address this defect. The attached patch is against the 10.6 branch,
but I would expect it to equally apply to the trunk. The description in the
initial report by Jeff Stuckman is accurate, please read it before studying the
patch.
Let me explain the patch, and the rationale behind it. Some familiarity with
the code in question is assumed. Four files where touched:
org.apache.derby.iapi.sql.PreparedStatement
org.apache.derby.impl.sql.GenericStatement
org.apache.derby.impl.sql.GenericActivationHolder
org.apache.derby.impl.sql.GenericPreparedStatement
Basically the previous code did this:
Block Thread B if it tries to recompile a statement that is being recompiled
by Thread A
This would result in deadlock if Thread A needs the exclusive (DB) lock held by
thread B.
Basically the new code does this:
When Thread B tries to recompile a statement AND that statement is currently
being recompiled (by another thread), don't block, simply create a new
GenericPreparedStatement and compile that instead. In essence, this is treated
like a "cache miss".
At the GenericStatement level, there is no way to know what Storage level locks
are being held. Therefore, blocking Thread B (via a Java lock) while awaiting
notification by a thread (Thread A) that might itself be waiting on a Storage
level lock held by Thread B is bound to lead to deadlocks in this, and possibly
other, scenarios.
This patch foregoes some [extremely] minor cache efficiencies by treating
concurrent recompilation of a statement as a cache miss. Recompilation by a
single thread is not treated as a cache miss, and the code flow is unchanged in
that case. Unless a statement undergoes constant concurrent recompilation
(defeating the statement cache anyway), I would expect this change to have an
almost immeasurable performance impact on applications. Certainly compared to
the workaround of disabling the statement cache, well, there is no comparison.
When studying this patch, I recommend reading in this order for clarity:
1. PreparedStatement
2. GenericPreparedStatement
3. GenericActivationHolder
4. GenericStatement
PreparedStatement:
The interface was changed so that rePrepare() might return a new statement.
GenericPreparedStatement:
In the "rePrepare()" implementation, account for the fact that a new
statement may be returned.
GenericActivationHolder:
In execute(), when a PreparedStatement is recognized as invalid and
therefore rePrepare()'d, accept the re-prepared statement and use it. 99% of
the time, the returned statement will be the same statement as the original.
But in the case of a concurrent re-preparation, a new statement will be
returned.
GenericStatement:
in prepMinion(), when we realize were in a "concurrent re-compile"
situation, rather than block waiting for the "other thread" to complete
recompiling the cached statement, simply create a new
GenericPreparedStatement() and compile it instead. This is where the rubber
meets the road, and the deadlock is avoided.
I ran the 'derbyall' test suite, and while I did receive two failures, they
seem unrelated to this fix and have more to do with my environment. I would
appreciate someone reviewing the patch, applying it to their system (with a
known passing test suite), and running it through a regression test pass.
> Statement cache deadlock
> ------------------------
>
> Key: DERBY-4279
> URL: https://issues.apache.org/jira/browse/DERBY-4279
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.0.2.1, 10.1.3.1, 10.2.2.0, 10.3.3.0, 10.4.2.0,
> 10.5.1.1
> Environment: Windows Vista
> Reporter: Jeff Stuckman
> Attachments: Derby4279.java
>
>
> Due to a design flaw in the statement cache, a deadlock can occur if a
> prepared statement becomes out-of-date.
> I will illustrate this with the following example:
> The application is using the embedded Derby driver. The application has two
> threads, and each thread uses its own connection.
> There is a table named MYTABLE with column MYCOLUMN.
> 1. A thread prepares and executes the query SELECT MYCOLUMN FROM MYTABLE. The
> prepared statement is stored in the statement cache (see
> org.apache.derby.impl.sql.GenericStatement for this logic)
> 2. After some time, the prepared statement becomes invalid or out-of-date for
> some reason (see org.apache.derby.impl.sql.GenericPreparedStatement)
> 3. Thread 1 begins a transaction and executes LOCK TABLE MYTABLE IN EXCLUSIVE
> MODE
> 4. Thread 2 begins a transaction and executes SELECT MYCOLUMN FROM MYTABLE.
> The statement is in the statement cache but it is out-of-date. The thread
> begins to recompile the statement. To compile the statement, the thread needs
> a shared lock on MYTABLE. Thread 1 already has an exclusive lock on MYTABLE.
> Thread 2 waits.
> 5. Thread 1 executes SELECT MYCOLUMN FROM MYTABLE. The statement is in the
> statement cache but it is being compiled. Thread 1 waits on the statement's
> monitor.
> 6. We have a deadlock. Derby eventually detects a lock timeout, but the error
> message is not descriptive. The stacks at the time of the deadlock are:
> This deadlock is unique because it can still occur in a properly designed
> database. You are only safe if all of your transactions are very simple and
> cannot be interleaved in a sequence that causes the deadlock, or if your
> particular statements do not require a table lock to compile. (For the sake
> of simplicity, I used LOCK TABLE in my example, but any UPDATE statement
> would fit.)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
