[
https://issues.apache.org/jira/browse/DERBY-633?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Matrigali updated DERBY-633:
---------------------------------
Urgency: Normal
Triaged July 2, 2009: assigned normal urgency.
A tool was run that marked possible problems, but no repro or report of an
actual problem.
> synchronization issues in client code
> -------------------------------------
>
> Key: DERBY-633
> URL: https://issues.apache.org/jira/browse/DERBY-633
> Project: Derby
> Issue Type: Bug
> Components: Network Client
> Affects 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.
-
You can reply to this email to add a comment to the issue online.
