Java: Can two threads access the same method safely if the access is read-only

Question

Preface

I have only written a couple of multithreaded programs in my day. It's usually once every two years when I have to do this. I'm trying to get more educated on the subject and I'm reading "Java Concurrency in Practice." I have a basic understanding.

Overview:

Typically, I never share objects across threads because it's easier and in most cases I'm just trying to avoid basic blocking scenarios. However, I have a use case where it makes sense to share an object across threads.

My JSONBmBindMappingRow is instantiated in my main thread (different class not included here). A private object BMBindMappingRow is set in JSONBmBindMappingRow . You can think of the JSONBmBindMappingRow class as immutable; although, it's definitely not. However, I will be treating it that way in my program.

After the JSONBmBindMappingRow is instantiated it can be assigned to multiple threads which will call getJsonRow().

Question:

The scope of my question is as follows: If two or more threads access the getJsonRow() at the sametime is this thread-safe since both will have a copy of the JSONBmBindMappingRow in there own memory cache? I think it's safe and synchronization is not needed, but I will leave it to the experts.

Is this code thread safe if two threads access it at the same time?

  public JSONRow getJsonRow()
  {
    JSONRow jrow = new JSONRow();
    for (Integer index: bbmr.getColumnMappingAll().keySet()) {
       BMFieldMapping bm = bbmr.getColumnMapping(index);
       if (bm.ws_field_name != null && !bm.ws_field_name.equalsIgnoreCase("")) {
          jrow.add(new JSONField(bm.ws_field_name, bm.getJavaDataType(), 1));
       }
    }
    return jrow;
  }

JSONBmBindMappingRow Class:

  package xxfi.oracle.apps.ws.json.row;

  import java.sql.Connection;
  import java.sql.SQLException;

  import oracle.jdbc.OracleCallableStatement;
  import oracle.jdbc.OracleResultSet;
  import oracle.jdbc.OracleTypes;

  import xxfi.oracle.apps.ws.bm.BMBindMappingRow;
  import xxfi.oracle.apps.ws.bm.BMFieldMapping;
  import xxfi.oracle.apps.ws.utility.JDBC;


  public class JSONBmBindMappingRow implements JSONRowBuildImpl
  {
     private BMBindMappingRow bbmr = new BMBindMappingRow();
     private Connection conn = null;
     private String tableName = null;
     private String className = this.getClass().getCanonicalName() ;

     public JSONBmBindMappingRow(Connection conn, String tableName)
     {
        this.tableName = tableName;
        this.conn = conn;
        init();
     }

     public void init()
     {
        setColumnBindMappings();
     }


     public void setColumnBindMappings()
     {
        StringBuffer plSql = new StringBuffer();

        plSql.append("DECLARE ");
        plSql.append("BEGIN ");
        plSql.append("   :1 := xxfi_bm_custom_table_api.get_column_binds ( ");
        plSql.append("  :2       /*tablename*/");
        plSql.append(");");
        plSql.append("END;");

        OracleCallableStatement oracleCallableStatement = null;
        OracleResultSet oracleResultSet = null;
        try {
           oracleCallableStatement = (OracleCallableStatement) this.conn.prepareCall(plSql.toString());
           oracleCallableStatement.registerOutParameter(1, OracleTypes.CURSOR);
           JDBC.nullSafe(oracleCallableStatement, 2, tableName, OracleTypes.VARCHAR);

           oracleCallableStatement.execute();

           // get cursor and cast it to ResultSet
           oracleResultSet = (OracleResultSet) oracleCallableStatement.getCursor(1);

           // loop it like normal
           while (oracleResultSet.next()) {
              bbmr.add(new BMFieldMapping(oracleResultSet.getString("ws_field_name"), 
                       oracleResultSet.getString("column_name"), oracleResultSet.getString("data_type"), 
                       oracleResultSet.getInt("bind_number")));

           }
         } catch (Exception e) {
           try {
              this.conn.rollback();
           } catch (SQLException f) {
              // TODO
           }
           System.out.println("Error in "+className+".setColumnBindMappings(): " + e);
           e.printStackTrace();

        } finally {
           JDBC.close(oracleCallableStatement, oracleResultSet);
        }
     }

     public String getArrayName()
     {
        return "";
     }

     public JSONRow getJsonRow()
     {
        JSONRow jrow = new JSONRow();
        for (Integer index: bbmr.getColumnMappingAll().keySet()) {
           BMFieldMapping bm = bbmr.getColumnMapping(index);
           if (bm.ws_field_name != null && !bm.ws_field_name.equalsIgnoreCase("")) {
              jrow.add(new JSONField(bm.ws_field_name, bm.getJavaDataType(), 1));
           }
        }
        return jrow;
     }

     public BMBindMappingRow getBbmr()
     {
        return bbmr;
     }
  }

Answer 1

If your access is read-only and no data is changed during access by the thread, by definition your threads will will work on the same data and no race condition can arise.

Answer 2

Assuming that getJsonRow() returns a JSONRow that is completely initialized with all data and which is never modified after that, then it is safe for all threads to retrieve data from this method concurrently.

Answer 3

No your code is not thread safe. The major problem is that you still need to provide synchronization or Java is under no obligation to make your writes visible.

During your ctor, you build a private object BMBindMappingRow. Java is under no obligation to make those writes visible to later reads by a different thread. If the ctor is called on one thread (one CPU/core) and then read by a different CPU on a different thread, you could have a big problem.

The basic idea is to synchronize somehow so that Java know it needs to make those writes visible. The easiest way give your code is to make the BMBindMappingRow final. Java guarantees at the end of the ctor there will be a synchronization event for the final field and all of its writes will be made visible.

The same idea applies to all of your fields (the instance variables). String is immutable and therefore thread safe. I believe Connection is thread safe too. However, the fields you assign in your class are not thread safe so you need to do something about that. You write to those fields in the ctor so again you must provide some form of synchronization. Again I think the easiest is just to make them all final.

final private BMBindMappingRow bbmr = new BMBindMappingRow();
final private Connection conn;
final private String tableName;
final private String className = this.getClass().getCanonicalName() ;

I encourage you to read up on the semantics of final fields. Chapter 17 of the Java language spec deals with multi-threading and is very readable. It's a bit thick in places but still something you should be able to plow through.

Note that it's super important using final this way to NEVER MODIFY the final field or the object it points to, or you break the synchronization that final provides. In that case you'll need to go back to synchronized or some classes from java.util.concurrent to provide visibility to your changes.

Since you mentioned class variables below in the comments, I'll get out the big guns and point you towards Java Concurrency in Practice by Brian Goetz. It is the book on Java concurrency and the only one I've found with no mistakes. It took me a couple of read throughs and questions here to fully grok it, but it's totally worth it to read the entire book carefully. Reading the spec is good too but this book explains things in much more detail and has useful examples too. Be a Java expert!