Provide a modicum of thread safety for marshaller init.

The _marshallers table must be guarded as computed marshallers can be created
at any time, and marshaller initialization itself must be guarded for the same
reason.

Thanks to Ray for pointing this out.
This commit is contained in:
Michael Bayne
2014-06-25 21:48:50 +00:00
parent c02625f0d7
commit 97b404c236
2 changed files with 24 additions and 30 deletions
@@ -181,7 +181,7 @@ public class PersistenceContext
* @param conprov provides JDBC {@link Connection} instances. * @param conprov provides JDBC {@link Connection} instances.
* @param adapter an optional adapter to a cache management system. * @param adapter an optional adapter to a cache management system.
*/ */
public void init (String ident, ConnectionProvider conprov, CacheAdapter adapter) public synchronized void init (String ident, ConnectionProvider conprov, CacheAdapter adapter)
{ {
// if we have no URL for the ident, freak out now; lest confusing failure happen later // if we have no URL for the ident, freak out now; lest confusing failure happen later
String url = conprov.getURL(ident); String url = conprov.getURL(ident);
@@ -267,10 +267,8 @@ public class PersistenceContext
Class<T> type, SchemaMigration migration) Class<T> type, SchemaMigration migration)
{ {
DepotMarshaller<T> marshaller = getRawMarshaller(type); DepotMarshaller<T> marshaller = getRawMarshaller(type);
if (marshaller.isInitialized()) { marshaller.requireInitialized(
throw new IllegalStateException( "Migrations must be registered before initializeRepositories() is called.");
"Migrations must be registered before initializeRepositories() is called.");
}
marshaller.registerMigration(migration); marshaller.registerMigration(migration);
} }
@@ -284,10 +282,9 @@ public class PersistenceContext
checkAreInitialized(); // le check du sanity checkAreInitialized(); // le check du sanity
DepotMarshaller<T> marshaller = getRawMarshaller(type); DepotMarshaller<T> marshaller = getRawMarshaller(type);
try { try {
if (!marshaller.isInitialized()) { // initialize the marshaller which may create or migrate the table for its underlying
// initialize the marshaller which may create or migrate the table for its // persistent object
// underlying persistent object if (marshaller.init(this, _meta)) {
marshaller.init(this, _meta);
if (marshaller.getTableName() != null && _warnOnLazyInit) { if (marshaller.getTableName() != null && _warnOnLazyInit) {
log.warning("Record initialized lazily", "type", type.getName(), log.warning("Record initialized lazily", "type", type.getName(),
new Exception()); new Exception());
@@ -568,10 +565,11 @@ public class PersistenceContext
/** /**
* Looks up and creates, but does not initialize, the marshaller for the specified Entity type. * Looks up and creates, but does not initialize, the marshaller for the specified Entity type.
*/ */
protected <T extends PersistentRecord> DepotMarshaller<T> getRawMarshaller (Class<T> type) protected synchronized <T extends PersistentRecord> DepotMarshaller<T> getRawMarshaller (
Class<T> type)
{ {
@SuppressWarnings("unchecked") DepotMarshaller<T> marshaller = @SuppressWarnings("unchecked") DepotMarshaller<T> marshaller = (DepotMarshaller<T>)
(DepotMarshaller<T>)_marshallers.get(type); _marshallers.get(type);
if (marshaller == null) { if (marshaller == null) {
_marshallers.put(type, marshaller = new DepotMarshaller<T>(type, this)); _marshallers.put(type, marshaller = new DepotMarshaller<T>(type, this));
} }
@@ -664,7 +662,7 @@ public class PersistenceContext
return invoke(op, false); return invoke(op, false);
} }
protected void checkAreInitialized () protected synchronized void checkAreInitialized ()
{ {
if (_conprov == null) { if (_conprov == null) {
throw new IllegalStateException( throw new IllegalStateException(
@@ -456,27 +456,22 @@ public class DepotMarshaller<T extends PersistentRecord> implements QueryMarshal
} }
/** /**
* Returns true if this marshaller has been initialized ({@link #init} has been called), its * Throws {@link IllegalStateException} with {@code msg} if this marshaller is not initialized.
* migrations run and it is ready for operation. False otherwise.
*/ */
public boolean isInitialized () public synchronized void requireInitialized (String msg) {
{ if (_meta == null) throw new IllegalStateException(msg);
return _meta != null;
} }
/** /**
* Initializes the table used by this marshaller. This is called automatically by the {@link * Initializes the table used by this marshaller if it has not yet been done. If the table does
* PersistenceContext} the first time an entity is used. If the table does not exist, it will * not exist, it will be created. If the schema version specified by the persistent object is
* be created. If the schema version specified by the persistent object is newer than the * newer than the database schema, it will be migrated.
* database schema, it will be migrated. * @return true if initialization was performed, false if it has already been performed.
*/ */
public void init (PersistenceContext ctx, DepotMetaData meta) public synchronized boolean init (PersistenceContext ctx, DepotMetaData meta)
throws DatabaseException throws DatabaseException
{ {
if (_meta != null) { // sanity check if (_meta != null) return false;
throw new IllegalStateException(
"Cannot re-initialize marshaller [type=" + _pClass + "].");
}
_meta = meta; _meta = meta;
final SQLBuilder builder = ctx.getSQLBuilder(new DepotTypes(ctx, _pClass)); final SQLBuilder builder = ctx.getSQLBuilder(new DepotTypes(ctx, _pClass));
@@ -488,7 +483,7 @@ public class DepotMarshaller<T extends PersistentRecord> implements QueryMarshal
// if we have no table (i.e. we're a computed entity), we have nothing to create // if we have no table (i.e. we're a computed entity), we have nothing to create
if (getTableName() == null) { if (getTableName() == null) {
return; return true;
} }
// figure out the list of fields that correspond to actual table columns and generate the // figure out the list of fields that correspond to actual table columns and generate the
@@ -522,7 +517,7 @@ public class DepotMarshaller<T extends PersistentRecord> implements QueryMarshal
if (Boolean.getBoolean("com.samskivert.depot.verifyschema")) { if (Boolean.getBoolean("com.samskivert.depot.verifyschema")) {
checkForStaleness(TableMetaData.load(ctx, getTableName()), ctx, builder); checkForStaleness(TableMetaData.load(ctx, getTableName()), ctx, builder);
} }
return; return true;
} }
// check whether migrations are allowed by our context // check whether migrations are allowed by our context
@@ -530,7 +525,7 @@ public class DepotMarshaller<T extends PersistentRecord> implements QueryMarshal
case WARN: case WARN:
log.warning(_pClass.getName() + " requires migration, which is disallowed. " + log.warning(_pClass.getName() + " requires migration, which is disallowed. " +
"Failures may be encountered later."); "Failures may be encountered later.");
return; return true;
case FAIL: case FAIL:
throw new DatabaseException( throw new DatabaseException(
_pClass.getName() + " requires migration, which is disallowed."); _pClass.getName() + " requires migration, which is disallowed.");
@@ -577,6 +572,7 @@ public class DepotMarshaller<T extends PersistentRecord> implements QueryMarshal
// and update our version in the schema version table // and update our version in the schema version table
_meta.updateVersion(getTableName(), _schemaVersion); _meta.updateVersion(getTableName(), _schemaVersion);
expectedDbVersion = _schemaVersion; expectedDbVersion = _schemaVersion;
return true;
} finally { } finally {
// set our migrating version back to zero // set our migrating version back to zero