db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mamta A. Satoor (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-3823) NullPointerException in stress.multi test
Date Fri, 07 Oct 2011 22:00:29 GMT

    [ https://issues.apache.org/jira/browse/DERBY-3823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123247#comment-13123247
] 

Mamta A. Satoor commented on DERBY-3823:
----------------------------------------

Hi Dag, Thanks for working on this jira. I spent some time on it and your patch. I want to
first recap my understanding of the issue and then share my thoughts on result set descriptor.
Please let me know if my understanding of the jira or your concern about result set descriptor
is incorrect

The problem seems to be that while getting the metadata by calling EmbedPreparedStatement.getMetaData(),we
can run into NPE because activation class assoicated with the prepared statement might be
NULL. Activation class is temporarily set to null while the prepared statement is getting
recompiled(GenericStatement.prepMinion) and it is possible that during that temporary NULL
activation stage, metadata code is trying to use the activation class. 

The proposed fix is to synchronize part of the logic in EmbedPreparedStatement.getMetaData().
The code that will be synchronized will be in a loop which will repreare the invalid prepared
statement and get it's activation class. If the activation class is null, then we will stay
in the while loop until we can grab a non-null activation class for the prepared statement.
When we get out of the synchronization code, we can rest assured that activation is not null.
After that, we pretty much follow the existing logic(with some code rework but the logic stays
the same) in EmbedPreparedStatement.getMetaData() 

The existing code in EmbedPreparedStatement.getMetaData() code is as follows 
                                if (preparedStatement.isValid() == false) 
                                { 
                                        //need to revalidate the statement here, otherwise
getResultDescription would 
                                        //still have info from previous valid statement 
                                        preparedStatement.rePrepare(lcc); 
                                        rMetaData = null; 
                                } 
                                //bug 4579 - gcDuringGetMetaData will be null if this is the
first time 
                                //getMetaData call is made. 
                                //Second check - if the statement was revalidated since last
getMetaData call, 
                                //then gcDuringGetMetaData wouldn't match with current generated
class name 
                                if (gcDuringGetMetaData == null || gcDuringGetMetaData.equals(execp.getActivationClass().getName())
== false) 
                                { 
                                        rMetaData = null; 
                                        gcDuringGetMetaData = execp.getActivationClass().getName();

                                } 
The code above can run into npe in "gcDuringGetMetaData = execp.getActivationClass().getName();"
 because getActivationClass() can be NULL. 

Part of the new suggested code is as follows 
+                synchronized(execp) { 
+                    // DERBY-3823 Some other thread may be repreparing 
+                    do { 
+                        while (!execp.upToDate()) { 
+                            execp.rePrepare(lcc); 
+                        } 
+ 
+                        currAc = execp.getActivationClass(); 
+                        resd = execp.getResultDescription(); 
+                    } while (currAc == null); 
+                } 
+ 
+                if (gcDuringGetMetaData == null) { 
+                    gcDuringGetMetaData = currAc.getName(); 
+                    rMetaData = null; 
+                } else { 
+                    if (!gcDuringGetMetaData.equals(currAc.getName())) { 
+                        rMetaData = null; 
+                        gcDuringGetMetaData = currAc.getName(); 
+                    } 
+                } 
+ 
With the new code, we can be certain that currAC is not null once we are outside the synchronized
code and hence we should not run into NPE later when we are accessing currAC

Later on, EmbedPreparedStatement.getMetaData() gets the resultDescription(existing code to
get the resultDescription is as follows ResultDescription resd = preparedStatement.getResultDescription();)
and new code from Dag is as follows (resd = execp.getResultDescription();) 

The concern raised here by Dag is that the resultDescription may not match the newly reprepared
statement's result description. I spent some time looking at the supporting code for getResultDescription
and I think we might be ok with the result description being in sync with preapred statement.
Following is what I found. Let me know if I might have missed something. 

getResultDescription() call from EmbedPreparedStatement.getMetaData() results into a call
to GenericPreparedStatement.getResultDescription() which simply return local variable resultDesc.
The resultDesc variable gets set by another method in the same class namely, GenericPreparedStatement.completeCompile.


The completeCompile method has following comment 
                // get the result description (null for non-cursor statements) 
                // would we want to reuse an old resultDesc? 
                // or do we need to always replace in case this was select *? 
                resultDesc = qt.makeResultDescription(); 
It appears from the comment in completeCompile  above that we don't simply return the existing
cached resultDesc(if it is not null). Instead, we actually reload the resultDesc everytime
by calling qt.makeResultDescription(). Based on this, I think result description and prepared
statement will not get out of sync. 
                
> NullPointerException in stress.multi test
> -----------------------------------------
>
>                 Key: DERBY-3823
>                 URL: https://issues.apache.org/jira/browse/DERBY-3823
>             Project: Derby
>          Issue Type: Bug
>          Components: Network Server
>    Affects Versions: 10.3.3.1, 10.7.1.1, 10.8.1.2
>            Reporter: Kathey Marsden
>              Labels: derby_triage10_5_2
>         Attachments: d3823-1.diff, derby.log
>
>
> I saw the following NPE in stress.multi running on 10.3 with derbyclient.
> java.lang.NullPointerException
>         at org.apache.derby.impl.jdbc.EmbedPreparedStatement.getMetaData(Unknown
>  Source)
>         at org.apache.derby.impl.drda.DRDAConnThread.writeSQLDARD(Unknown Source
> )
>         at org.apache.derby.impl.drda.DRDAConnThread.processCommands(Unknown Sou
> rce)
>         at org.apache.derby.impl.drda.DRDAConnThread.run(Unknown Source)
> Cleanup action completed

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message