Derby
  1. Derby
  2. DERBY-4973

NullPointerException in updatelocks.sql encryption tests on IBM 1.6

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.8.1.2
    • Component/s: Store
    • Labels:
      None
    • Environment:
    • Issue & fix info:
      High Value Fix
    • Bug behavior facts:
      Regression Test Failure

      Description

      The IBM 1.6 results for Jan 13, 2011 show the following failure for IBM 1.6 on XP

          • End: TestEnc jdk1.6.0 DerbyNetClient derbynetclientmats:encodingTests 2011-01-14 00:05:38 ***
                      • Diff file derbyall/encryptionAll/storemats/storemats/updatelocks.diff
          • Start: updatelocks jdk1.6.0 storemats:storemats 2011-01-14 00:18:37 ***
            19859a19860,19868
            > ERROR 38000: The exception 'java.lang.NullPointerException' was thrown while evaluating an expression.
            > ERROR XJ001: Java exception: ': java.lang.NullPointerException'.
            > ij> next scan_cursor;
            > A |B |C
            > --------------------------------------------------------------------------------------------------------------------------------------------------------
            > -3 |-30 |-three
            > ij> update a set a=3,b=30,c='three' where current of scan_cursor;
            > 1 row inserted/updated/deleted
            > ij> select * from lock_table order by tabname, type desc, mode, cnt, lockname;
            19861a19871
            > APP |UserTran|TABLE |1 |IX |A |Tablelock |GRANT|ACTIVE
            19866,19876d19875
            < -3 |-30 |-three
            < ij> update a set a=3,b=30,c='three' where current of scan_cursor;
            < 1 row inserted/updated/deleted
            < ij> select * from lock_table order by tabname, type desc, mode, cnt, lockname;
            < USERNAME|TRANTYPE|TYPE |CNT |MODE|TABNAME |LOCKNAME |STATE|STATUS
            < ---------------------------------------------------------------------------
            < APP |UserTran|TABLE |1 |IX |A |Tablelock |GRANT|ACTIVE
            < APP |UserTran|TABLE |1 |X |A |Tablelock |GRANT|ACTIVE
            < ij> next scan_cursor;
            < A |B |C
            < --------------------------------------------------------------------------------------------------------------------------------------------------------
            Test Failed.
          • End: updatelocks jdk1.6.0 storemats:storemats 2011-01-14 00:19:24 ***
            ------------------------------------------------------

      Stack trace is:
      Fri Jan 14 00:19:08 PST 2011 Thread[main,5,main] (XID = 2648), (SESSIONID = 1), (DATABASE = wombat), (DRDAID = null), Failed Statement is: select * from lock_table order by tabname, type desc, mode, cnt, lockname

      ERROR 38000: The exception 'java.lang.NullPointerException' was thrown while evaluating an expression.

      at org.apache.derby.iapi.error.StandardException.newException(Unknown Source)

      at org.apache.derby.iapi.error.StandardException.unexpectedUserException(Unknown Source)

      at org.apache.derby.impl.sql.execute.VTIResultSet.populateFromResultSet(Unknown Source)

      at org.apache.derby.impl.sql.execute.VTIResultSet.getNextRowCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.HashTableResultSet.getNextRowFromRowSource(Unknown Source)

      at org.apache.derby.iapi.store.access.BackingStoreHashtable.getNextRowFromRowSource(Unknown Source)

      at org.apache.derby.iapi.store.access.BackingStoreHashtable.<init>(Unknown Source)

      at org.apache.derby.impl.sql.execute.HashTableResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.ProjectRestrictResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.JoinResultSet.openRight(Unknown Source)

      at org.apache.derby.impl.sql.execute.JoinResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.ProjectRestrictResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.SortResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.BasicNoPutResultSetImpl.open(Unknown Source)

      at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)

      at org.apache.derby.impl.sql.GenericPreparedStatement.execute(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)

      at org.apache.derby.impl.tools.ij.ij.executeImmediate(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.doCatch(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.go(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.go(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.mainCore(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.main(Unknown Source)

      at org.apache.derby.tools.ij.main(Unknown Source)

      Caused by: java.lang.NullPointerException

      at org.apache.derby.impl.store.raw.xact.Xact.getContextId(Unknown Source)

      at org.apache.derby.impl.store.raw.xact.TransactionTableEntry.getTransactionTypeString(Unknown Source)

      at org.apache.derby.diag.TransactionTable.getString(Unknown Source)

      at org.apache.derby.iapi.types.SQLChar.setValueFromResultSet(Unknown Source)

      ... 25 more

      ============= begin nested exception, level (1) ===========

      java.lang.NullPointerException

      at org.apache.derby.impl.store.raw.xact.Xact.getContextId(Unknown Source)

      at org.apache.derby.impl.store.raw.xact.TransactionTableEntry.getTransactionTypeString(Unknown Source)

      at org.apache.derby.diag.TransactionTable.getString(Unknown Source)

      at org.apache.derby.iapi.types.SQLChar.setValueFromResultSet(Unknown Source)

      at org.apache.derby.impl.sql.execute.VTIResultSet.populateFromResultSet(Unknown Source)

      at org.apache.derby.impl.sql.execute.VTIResultSet.getNextRowCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.HashTableResultSet.getNextRowFromRowSource(Unknown Source)

      at org.apache.derby.iapi.store.access.BackingStoreHashtable.getNextRowFromRowSource(Unknown Source)

      at org.apache.derby.iapi.store.access.BackingStoreHashtable.<init>(Unknown Source)

      at org.apache.derby.impl.sql.execute.HashTableResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.ProjectRestrictResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.JoinResultSet.openRight(Unknown Source)

      at org.apache.derby.impl.sql.execute.JoinResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.ProjectRestrictResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.SortResultSet.openCore(Unknown Source)

      at org.apache.derby.impl.sql.execute.BasicNoPutResultSetImpl.open(Unknown Source)

      at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)

      at org.apache.derby.impl.sql.GenericPreparedStatement.execute(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)

      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)

      at org.apache.derby.impl.tools.ij.ij.executeImmediate(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.doCatch(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(Unknown Source)

      at org.apache.derby.impl.tools.ij.utilMain.go(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.go(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.mainCore(Unknown Source)

      at org.apache.derby.impl.tools.ij.Main.main(Unknown Source)

      at org.apache.derby.tools.ij.main(Unknown Source)

      ============= end nested exception, level (1) ===========

      1. derby-4973_diff.txt
        1 kB
        Kathey Marsden
      2. updatelocks_results.zip
        942 kB
        Kathey Marsden

        Activity

        Hide
        Kathey Marsden added a comment -

        Attaching full test results.

        Show
        Kathey Marsden added a comment - Attaching full test results.
        Hide
        Kathey Marsden added a comment -

        This is the method with the NPE:
        Get my transaction context Id
        */
        public final String getContextId()

        { return (xc == null) ? null : xc.getIdName(); }

        I think this might be some sort of inlining JVM issue as it seems prepared for xc to be null.

        Show
        Kathey Marsden added a comment - This is the method with the NPE: Get my transaction context Id */ public final String getContextId() { return (xc == null) ? null : xc.getIdName(); } I think this might be some sort of inlining JVM issue as it seems prepared for xc to be null.
        Hide
        Kathey Marsden added a comment -

        I checked with Mike Matrigali to double check that if it might be possible that the xc field could be nulled out after the check but before the dereference in the line
        return (xc == null) ? null : xc.getIdName();

        The particular case of failure of a locktable query, the query is looking at the transaction context of other transactions in flux, so that might actually be possible. I am thinking maybe the reason we see this with the new JVM might be that perhaps the compiler was changes to do two reads of the field in this case instead of one. I am going to put in some sleeps to try to expose the issue and then ultimately per Mike's suggestion, recode the method to avoid the double read, like:

        tempxc = xc;
        return (tempxc == null) ? null : tempxc.getIdName();

        That way we can avoid adding synchronization to this low level code for the benefit of the locktable query and ensure just one read.

        Show
        Kathey Marsden added a comment - I checked with Mike Matrigali to double check that if it might be possible that the xc field could be nulled out after the check but before the dereference in the line return (xc == null) ? null : xc.getIdName(); The particular case of failure of a locktable query, the query is looking at the transaction context of other transactions in flux, so that might actually be possible. I am thinking maybe the reason we see this with the new JVM might be that perhaps the compiler was changes to do two reads of the field in this case instead of one. I am going to put in some sleeps to try to expose the issue and then ultimately per Mike's suggestion, recode the method to avoid the double read, like: tempxc = xc; return (tempxc == null) ? null : tempxc.getIdName(); That way we can avoid adding synchronization to this low level code for the benefit of the locktable query and ensure just one read.
        Hide
        Dag H. Wanvik added a comment -

        Perhaps we should also make xc volatile, so the reader is forced to refresh the value from memory?

        Show
        Dag H. Wanvik added a comment - Perhaps we should also make xc volatile, so the reader is forced to refresh the value from memory?
        Hide
        Kathey Marsden added a comment -

        Dag could you elaborate on the benefit of making xc volatile?

        I guess one good thing is that it would be self documenting that it might be changed by other threads, but actually in this case it would have been nice for a thread local copy to be made so it wouldn't change. Perhaps that is the difference we are seeing between SR8 and SR9 is that now it is refreshing the value from memory, whereas before it was not? I guess though that absence of volatile means that it can cache a thread local copy, but is not a requirement that it does.

        Show
        Kathey Marsden added a comment - Dag could you elaborate on the benefit of making xc volatile? I guess one good thing is that it would be self documenting that it might be changed by other threads, but actually in this case it would have been nice for a thread local copy to be made so it wouldn't change. Perhaps that is the difference we are seeing between SR8 and SR9 is that now it is refreshing the value from memory, whereas before it was not? I guess though that absence of volatile means that it can cache a thread local copy, but is not a requirement that it does.
        Hide
        Dag H. Wanvik added a comment - - edited

        Ah, yes, I guess I misunderstood what you are trying to achieve here. I thought that the correct behaviour would be to actually see the change by another thread, but in a thread safe way. If you need a thread local copy, then adding volatile wouldn't help surely. But yes, if volatile is not used, the thread is not forced to reread from memory if the compiler knows it hasn't been changed by the current thread. Or that's how I understand it anyway[1]

        [1] http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#volatile

        Show
        Dag H. Wanvik added a comment - - edited Ah, yes, I guess I misunderstood what you are trying to achieve here. I thought that the correct behaviour would be to actually see the change by another thread, but in a thread safe way. If you need a thread local copy, then adding volatile wouldn't help surely. But yes, if volatile is not used, the thread is not forced to reread from memory if the compiler knows it hasn't been changed by the current thread. Or that's how I understand it anyway [1] [1] http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#volatile
        Hide
        Kathey Marsden added a comment -

        I think then that I won't make it volatile and will add the tempxc code to make sure it is just read once and is stable for the duration of the getContextId() call.

        Note: To pop the bug more reliably, I changed the code to have a sleep between the check and the dereference:
        public final String getContextId()
        {
        //return (xc == null) ? null : xc.getIdName();
        if (xc == null)
        return null;
        else {
        try

        { Thread.currentThread().sleep(100); }

        catch (InterruptedException e)

        { // TODO Auto-generated catch block e.printStackTrace(); }

        return xc.getIdName();
        }
        }

        With this change I see the NullPointerException on SR8 as well as Sun JDK 1.6, so I think this is definitely a Derby bug and not a JVM issue as I first thought. There may be SR9 changes so that they don't use a thread local copy where they can, but I don't think that they are required to do so, they just have that option.

        Show
        Kathey Marsden added a comment - I think then that I won't make it volatile and will add the tempxc code to make sure it is just read once and is stable for the duration of the getContextId() call. Note: To pop the bug more reliably, I changed the code to have a sleep between the check and the dereference: public final String getContextId() { //return (xc == null) ? null : xc.getIdName(); if (xc == null) return null; else { try { Thread.currentThread().sleep(100); } catch (InterruptedException e) { // TODO Auto-generated catch block e.printStackTrace(); } return xc.getIdName(); } } With this change I see the NullPointerException on SR8 as well as Sun JDK 1.6, so I think this is definitely a Derby bug and not a JVM issue as I first thought. There may be SR9 changes so that they don't use a thread local copy where they can, but I don't think that they are required to do so, they just have that option.
        Hide
        Kathey Marsden added a comment -

        Here is the patch to ensure just a single read of xc in Xact.getContextId(). Running tests...

        Show
        Kathey Marsden added a comment - Here is the patch to ensure just a single read of xc in Xact.getContextId(). Running tests...
        Hide
        Kathey Marsden added a comment -

        ported this back as far as 10.3. I am sure it exists back to 10.1, but won't bother for now.

        Show
        Kathey Marsden added a comment - ported this back as far as 10.3. I am sure it exists back to 10.1, but won't bother for now.
        Hide
        Knut Anders Hatlen added a comment -

        [bulk update] Close all resolved issues that haven't been updated for more than one year.

        Show
        Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

          People

          • Assignee:
            Kathey Marsden
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development