db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mayur Naik (JIRA)" <derby-...@db.apache.org>
Subject [jira] Created: (DERBY-633) synchronization issues in client code
Date Wed, 19 Oct 2005 21:20:46 GMT
synchronization issues in client code
-------------------------------------

         Key: DERBY-633
         URL: http://issues.apache.org/jira/browse/DERBY-633
     Project: Derby
        Type: Bug
  Components: Network Client  
    Versions: 10.1.1.0    
    Reporter: Mayur Naik


I ran a static race detection tool to check the implementation of the following java.sql.*
interfaces in Derby and found 319 possible bugs (static instances of missing synchronization):

Blob
Clob
DatabaseMetaData
Connection
Statement
PreparedStatement
CallableStatement
ResultSet

I ran the tool in 2 passes.  The results of the 2 passes are here:

http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/

PASS1 reports "shallow" races, which I fixed and re-ran the tool, and PASS2 reports any remaining
deep races.  I fixed those as well and re-ran the tool, and it did not report any more races.
 The 2 passes are because a single instance of bad synchronization can cause multiple races
(and, conversely, a single race can indicate multiple instances of bad synchronization), so
to prevent overwhelming the user with race reports, the first pass checks only for races on
fields of top-level objects (ex. NetConnection) but the second pass does a full-blown check.

====================================================================
Analysis of results of PASS 1:
====================================================================

===============================
java.sql.Blob

0 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/blob/index.html

===============================
java.sql.Clob

0 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/clob/index.html

===============================
java.sql.Connection

147 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/connection/index.html

These are due to lack of synchronization on 'this' in 12 methods in class Connection:

boolean getAutoCommit()
boolean isClosed()
int getTransactionIsolation()
java.sql.SQLWarning getWarnings()
java.sql.DatabaseMetaData getMetaData()
boolean isReadOnly()
String getCatalog()
java.util.Map getTypeMap()
int getHoldability()
java.sql.PreparedStatement prepareStatement(String sql, int autoGeneratedKeys)
java.sql.PreparedStatement prepareStatement(String sql, int columnIndexes[])
java.sql.PreparedStatement prepareStatement(String sql, String columnNames[])

In most of the above cases, it is the classic situation of possibly incorrect synchronization
in which the programmer leaves get* methods unsychronized.  It may or may not be correct depending
upon what those get* methods are doing.  I leave it to the programmer to decide in this case.
 Also, it is not sufficient to reason at the Java level: given the subtleties of the Java
memory model, one needs to reason at the bytecode level (see the next item for more on this).

===============================
java.sql.DatabaseMetaData

6 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/database_metadata/index.html

All are on field metaDataInfoIsCached_ of class DatabaseMetaData due to lack of synchronization
on connection_ in 107 public methods in this class that call a getMetaData* method.  One of
these 107 methods is:

    public boolean supportsBatchUpdates() throws SqlException {
        checkForClosedConnection();
        return getMetaDataInfoBoolean(supportsBatchUpdates__);
    }

The programmer seems to be aware of these races b/w the protected write access to field metaDataInfoIsCached_
in the below method

    // We synchronize at this level so that we don't have to synchronize all
    // the meta data info methods.  If we just return hardwired answers we
    // don't need to synchronize at the higher level.
    private void metaDataInfoCall() {
        synchronized (connection_) {
            ... // update metaDataInfoCache_
            metaDataInfoIsCached_ = true;
        }
    }

and the unprotected read access to that field in each of the getMetaData* methods, for instance:

    private String getMetaDataInfoString(int infoCallIndex) {
        if (metaDataInfoIsCached_) {
            return (String) metaDataInfoCache_[infoCallIndex];
        } 
        ...
    }

The races are benign if one reasons at the Java level, but they might be worth looking at
from the perspective of the bytecode level.  In particular, since a JVM is free to reorder
instructions within a synchronized block, it is legal (though highly unlikely) for it to move
the write to metaDataInfoIsCached_ in the metaDataInfoCall method to before the "..." code
that updates the elements of array metaDataInfoCache_, in which case there is clearly a bug.
 I myself am not very familiar with the Java memory model, but see http://www.cs.umd.edu/users/pugh/java/memoryModel/jsr-133-faq.html
for some unexpected things that can happen due to reordering of bytecode which is legal within
(but not across) a synchronization block.

===============================
java.sql.Statement

20 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/statement/index.html

These are due to lack of synchronization on connection_ in 14 methods in class Statement:

int getMaxFieldSize()
int getMaxRows()
int getQueryTimeout()
void cancel()
java.sql.SQLWarning getWarnings() 
int getFetchDirection()
int getFetchSize() 
int getResultSetConcurrency()
int getResultSetType()
java.sql.Connection getConnection() 
java.sql.ResultSet getGeneratedKeys() 
int executeUpdate(String sql, int columnIndexes[]) 
boolean execute(String sql, int columnIndexes[])
int getResultSetHoldability() 

Most of these are due to unsynchronized get* methods (similar to Connection above).

===============================
java.sql.PreparedStatement

0 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/prepared_statement/index.html

===============================
java.sql.CallableStatement

26 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/callable_statement/index.html

All are on field wasNull_ due to lack of synchronization on connection_ in the wasNull method
in class CallableStatement.

===============================
java.sql.ResultSet

764 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS1/result_set/index.html

These are due to lack of synchronization on connection_ in 87 methods in class ResultSet:

boolean wasNull()
boolean getBoolean(int column)
byte getByte(int column)
short getShort(int column)
int getInt(int column)
long getLong(int column)
float getFloat(int column)
double getDouble(int column)
java.math.BigDecimal getBigDecimal(int column, int scale)
java.math.BigDecimal getBigDecimal(int column)
java.sql.Date getDate(int column)
java.sql.Date getDate(int column, java.util.Calendar calendar)
java.sql.Time getTime(int column)
java.sql.Time getTime(int column, java.util.Calendar calendar)
java.sql.Timestamp getTimestamp(int column)
java.sql.Timestamp getTimestamp(int column, java.util.Calendar calendar)
String getString(int column)
byte[] getBytes(int column)
java.io.InputStream getBinaryStream(int column)
java.io.InputStream getAsciiStream(int column)
java.io.InputStream getUnicodeStream(int column)
java.io.Reader getCharacterStream(int column)
java.sql.Blob getBlob(int column)
java.sql.Clob getClob(int column)
java.sql.Ref getRef(int column)
java.sql.Array getArray(int column)
Object getObject(int column)
Object getObject(int column, java.util.Map map)
final boolean getBoolean(String columnName)
final byte getByte(String columnName)
final short getShort(String columnName)
final int getInt(String columnName)
final long getLong(String columnName)
final float getFloat(String columnName)
final double getDouble(String columnName)
final java.math.BigDecimal getBigDecimal(String columnName, int scale)
final java.math.BigDecimal getBigDecimal(String columnName)
final java.sql.Date getDate(String columnName)
final java.sql.Date getDate(String columnName, java.util.Calendar cal)
final java.sql.Time getTime(String columnName)
final java.sql.Time getTime(String columnName, java.util.Calendar cal)
final java.sql.Timestamp getTimestamp(String columnName)
final java.sql.Timestamp getTimestamp(String columnName, java.util.Calendar cal)
final String getString(String columnName)
final byte[] getBytes(String columnName)
final java.io.InputStream getBinaryStream(String columnName)
final java.io.InputStream getAsciiStream(String columnName)
final java.io.InputStream getUnicodeStream(String columnName)
final java.io.Reader getCharacterStream(String columnName)
final java.sql.Blob getBlob(String columnName)
final java.sql.Clob getClob(String columnName)
final java.sql.Array getArray(String columnName)
final java.sql.Ref getRef(String columnName)
final Object getObject(String columnName)
final Object getObject(String columnName, java.util.Map map)
final java.sql.SQLWarning getWarnings()
java.sql.ResultSetMetaData getMetaData()
boolean isBeforeFirst()
boolean isAfterLast()
boolean isFirst()
boolean isLast()
int getFetchDirection()
int getFetchSize()
int getType()
int getConcurrency()
boolean rowUpdated()
boolean rowInserted()
boolean rowDeleted()
void updateNull(String columnName)
void updateBoolean(String columnName, boolean x)
void updateByte(String columnName, byte x)
void updateShort(String columnName, short x)
void updateInt(String columnName, int x)
void updateLong(String columnName, long x)
void updateFloat(String columnName, float x)
void updateDouble(String columnName, double x)
void updateBigDecimal(String columnName, java.math.BigDecimal x)
void updateDate(String columnName, java.sql.Date x)
void updateTime(String columnName, java.sql.Time x)
void updateTimestamp(String columnName, java.sql.Timestamp x)
void updateString(String columnName, String x)
void updateBytes(String columnName, byte x[])
void updateBinaryStream(String columnName, java.io.InputStream x, int length)
void updateAsciiStream(String columnName, java.io.InputStream x, int length)
void updateCharacterStream(String columnName, java.io.Reader x, int length)
void updateObject(String columnName, Object x, int scale)
void updateObject(String columnName, Object x)

Most of these races seem to be intentional, for instance, many (but not all) of them contain
the following comments:

    // Live life on the edge and run unsynchronized
    public boolean getBoolean(int column) {
        ...
        setWasNull(column);  // Placed close to the return to minimize risk of thread interference
        return result;
    }

But as I said above, it might be worth looking at these races because of unexpected transformations
that the JVM might do at the bytecode level.  Clearly, the second comment in the above method
suggests that the programmer assumes absence of aggressive transformations.

====================================================================
Analysis of results of PASS 2:
====================================================================

===============================
java.sql.Blob

0 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/blob/index.html

===============================
java.sql.Clob

0 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/clob/index.html

===============================
java.sql.Connection

51 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/connection/index.html

These races are on fields so deep inside some class (java.nio.Buffer) that I cannot verify
them.  But synchronizing method nativeSQL in class Connection on 'this' eliminates them.

===============================
java.sql.ResultSet

51 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/result_set/

Similar to the ones reported for Connection above.  Synchronizing method getStatement in class
ResultSet on connection_ eliminates them.

===============================
java.sql.Statement

51 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/statement/index.html

Similar to the ones reported for Connection above.  They are eliminated once the races reported
below for PreparedStatement are eliminated.

===============================
java.sql.PreparedStatement

51 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/prepared_statement/index.html

Similar to the ones reported for Connection above.  Synchronizing 10 methods in class PreparedStatement
on connection_ eliminates them:

boolean execute(String sql)
java.sql.ResultSet executeQuery(String sql)
int executeUpdate(String sql)
boolean execute(String sql, int autoGeneratedKeys)
boolean execute(String sql, String[] columnNames) 
boolean execute(String sql, int[] columnIndexes)
int executeUpdate(String sql, int autoGeneratedKeys) 
int executeUpdate(String sql, String[] columnNames)
int executeUpdate(String sql, int[] columnIndexes)
void setURL(int parameterIndex, java.net.URL x) 

===============================
java.sql.DatabaseMetaData

3 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/database_metadata/index.html

All are on field deferredException_ of class Agent, due to lack of synchronization on connection_
in 34 methods in class DatabaseMetaData:

boolean supportsSavepoints()
String getURL()
String getUserName()
String getDatabaseProductName()
String getDatabaseProductVersion()
String getDriverName()
String getDriverVersion()
boolean supportsMixedCaseIdentifiers()
boolean supportsMixedCaseQuotedIdentifiers()
String getIdentifierQuoteString()
boolean supportsColumnAliasing()
boolean nullPlusNonNullIsNull()
boolean supportsTableCorrelationNames()
boolean supportsLikeEscapeClause()
boolean supportsNonNullableColumns()
boolean supportsMinimumSQLGrammar()
boolean supportsANSI92EntryLevelSQL()
boolean supportsSubqueriesInExists()
boolean supportsSubqueriesInIns()
boolean supportsSubqueriesInQuantifieds()
boolean supportsCorrelatedSubqueries()
java.sql.Connection getConnection()
boolean supportsNamedParameters()
boolean supportsMultipleOpenResults()
boolean supportsGetGeneratedKeys()
boolean supportsResultSetHoldability(int holdability)
int getResultSetHoldability()
int getDatabaseMajorVersion()
int getDatabaseMinorVersion()
int getJDBCMajorVersion()
int getJDBCMinorVersion()
int getSQLStateType()
boolean locatorsUpdateCopy()
boolean supportsStatementPooling()

The races arise because all these methods call checkForClosedConnection defined in the same
class which in turn calls checkForDeferredExceptions in class Agent:

    void checkForDeferredExceptions() throws SqlException {
        if (deferredException_ != null) {
            SqlException temp = deferredException_;
            deferredException_ = null;
            throw temp;
        }
   }

These races can be fixed by simply removing the call to checkForClosedConnection in most of
the above methods, instead of adding synchronization to them.  In particular, this method
doesn't call it:

   // JDBC signature also does not throw SqlException, so we don't check for
   // closed connection.
    public int getDriverMajorVersion() {
        return Version.getMajorVersion();
    }

Then why does a trivial method like this one call it?

    public String getIdentifierQuoteString() throws SqlException {
        checkForClosedConnection();
        return "\"";
    }

Most of the above methods are trivial, simply returning a constant value.  Do you really need
to call checkForClosedConnection in them?

===============================
java.sql.CallableStatement

3 races:
http://suif.stanford.edu/~mhn/db-derby-10.1.1.0-src/classes/PASS2/callable_statement/index.html

All are on field deferredException_ of class Agent, due to lack of synchronization on connection_
in 52 methods in class CallableStatement (one of which is shown below):

    public java.net.URL getURL(String parameterName) throws SqlException {
        if (agent_.loggingEnabled()) {
            agent_.logWriter_.traceEntry(this, "getURL", parameterName);
        }
        super.checkForClosedStatement();
        throw new SqlException(agent_.logWriter_, "JDBC 3 method called - not yet supported");
    }

The above method calls checkForClosedStatement in class Statement which in turn calls checkForDeferredExceptions
in class Agent.


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message