diff options
| -rw-r--r-- | chrome/browser/sync/engine/syncapi.cc | 39 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/syncapi.h | 7 | ||||
| -rw-r--r-- | chrome/browser/sync/syncable/syncable.cc | 11 | ||||
| -rw-r--r-- | chrome/browser/sync/syncable/syncable.h | 10 |
4 files changed, 52 insertions, 15 deletions
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index b1678e8..d9c5a9f 100644 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -770,12 +770,21 @@ bool ReadNode::InitByTagLookup(const std::string& tag) { // ReadTransaction member definitions ReadTransaction::ReadTransaction(UserShare* share) : BaseTransaction(share), - transaction_(NULL) { + transaction_(NULL), + close_transaction_(true) { transaction_ = new syncable::ReadTransaction(GetLookup(), __FILE__, __LINE__); } +ReadTransaction::ReadTransaction(UserShare* share, + syncable::BaseTransaction* trans) + : BaseTransaction(share), + transaction_(trans), + close_transaction_(false) {} + ReadTransaction::~ReadTransaction() { - delete transaction_; + if (close_transaction_) { + delete transaction_; + } } syncable::BaseTransaction* ReadTransaction::GetWrappedTrans() const { @@ -923,7 +932,7 @@ class SyncManager::SyncInternal // builds the list of sync-engine initiated changes that will be forwarded to // the SyncManager's Observers. virtual void HandleChannelEvent(const syncable::DirectoryChangeEvent& event); - void HandleTransactionCompleteChangeEvent( + void HandleTransactionEndingChangeEvent( const syncable::DirectoryChangeEvent& event); void HandleCalculateChangesChangeEventFromSyncApi( const syncable::DirectoryChangeEvent& event); @@ -1143,8 +1152,8 @@ class SyncManager::SyncInternal // Each element of this array is a store of change records produced by // HandleChangeEvent during the CALCULATE_CHANGES step. The changes are // segregated by model type, and are stored here to be processed and - // forwarded to the observer slightly later, at the TRANSACTION_COMPLETE - // step by HandleTransactionCompleteChangeEvent. + // forwarded to the observer slightly later, at the TRANSACTION_ENDING + // step by HandleTransactionEndingChangeEvent. ChangeReorderBuffer change_buffers_[syncable::MODEL_TYPE_COUNT]; // The event listener hookup that is registered for HandleChangeEvent. @@ -1635,8 +1644,8 @@ void SyncManager::SyncInternal::HandleDirectoryManagerEvent( // ApplyUpdates) to data_->changelist. void SyncManager::SyncInternal::HandleChannelEvent( const syncable::DirectoryChangeEvent& event) { - if (event.todo == syncable::DirectoryChangeEvent::TRANSACTION_COMPLETE) { - HandleTransactionCompleteChangeEvent(event); + if (event.todo == syncable::DirectoryChangeEvent::TRANSACTION_ENDING) { + HandleTransactionEndingChangeEvent(event); return; } else if (event.todo == syncable::DirectoryChangeEvent::CALCULATE_CHANGES) { if (event.writer == syncable::SYNCAPI) { @@ -1650,15 +1659,21 @@ void SyncManager::SyncInternal::HandleChannelEvent( } } -void SyncManager::SyncInternal::HandleTransactionCompleteChangeEvent( +void SyncManager::SyncInternal::HandleTransactionEndingChangeEvent( const syncable::DirectoryChangeEvent& event) { - // This notification happens immediately after a syncable WriteTransaction - // falls out of scope. - DCHECK_EQ(event.todo, syncable::DirectoryChangeEvent::TRANSACTION_COMPLETE); + // This notification happens immediately before a syncable WriteTransaction + // falls out of scope. It happens while the channel mutex is still held, + // and while the transaction mutex is held, so it cannot be re-entrant. + DCHECK_EQ(event.todo, syncable::DirectoryChangeEvent::TRANSACTION_ENDING); if (!observer_ || ChangeBuffersAreEmpty()) return; - ReadTransaction trans(GetUserShare()); + // This will continue the WriteTransaction using a read only wrapper. + // This is the last chance for read to occur in the WriteTransaction + // that's closing. This special ReadTransaction will not close the + // underlying transaction. + ReadTransaction trans(GetUserShare(), event.trans); + for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) { if (change_buffers_[i].IsEmpty()) continue; diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h index bb90976..90e1a5c 100644 --- a/chrome/browser/sync/engine/syncapi.h +++ b/chrome/browser/sync/engine/syncapi.h @@ -474,6 +474,10 @@ class ReadTransaction : public BaseTransaction { public: // Start a new read-only transaction on the specified repository. explicit ReadTransaction(UserShare* share); + + // Resume the middle of a transaction. Will not close transaction. + ReadTransaction(UserShare* share, syncable::BaseTransaction* trans); + virtual ~ReadTransaction(); // BaseTransaction override. @@ -482,7 +486,8 @@ class ReadTransaction : public BaseTransaction { void* operator new(size_t size); // Transaction is meant for stack use only. // The underlying syncable object which this class wraps. - syncable::ReadTransaction* transaction_; + syncable::BaseTransaction* transaction_; + bool close_transaction_; DISALLOW_COPY_AND_ASSIGN(ReadTransaction); }; diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc index 7c6cc71..5516c59 100644 --- a/chrome/browser/sync/syncable/syncable.cc +++ b/chrome/browser/sync/syncable/syncable.cc @@ -950,6 +950,8 @@ BaseTransaction::BaseTransaction(Directory* directory, const char* name, Lock(); } +BaseTransaction::~BaseTransaction() {} + void BaseTransaction::UnlockAndLog(OriginalEntries* originals_arg) { dirkernel_->transaction_mutex.AssertAcquired(); @@ -972,8 +974,17 @@ void BaseTransaction::UnlockAndLog(OriginalEntries* originals_arg) { originals.get(), this, writer_ }; dirkernel_->changes_channel.Notify(event); + // Necessary for reads to be performed prior to transaction mutex release. + // Allows the listener to use the current transaction to perform reads. + DirectoryChangeEvent ending_event = + { DirectoryChangeEvent::TRANSACTION_ENDING, + NULL, this, INVALID }; + dirkernel_->changes_channel.Notify(ending_event); + dirkernel_->transaction_mutex.Release(); + // Directly after transaction mutex release, but lock on changes channel. + // You cannot be re-entrant to a transaction in this handler. DirectoryChangeEvent complete_event = { DirectoryChangeEvent::TRANSACTION_COMPLETE, NULL, NULL, INVALID }; diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h index 3793a2c..291e798 100644 --- a/chrome/browser/sync/syncable/syncable.h +++ b/chrome/browser/sync/syncable/syncable.h @@ -586,6 +586,10 @@ struct DirectoryChangeEvent { // callbacks or attempt to lock anything because a // WriteTransaction is being held until the listener returns. CALCULATE_CHANGES, + // Means the WriteTransaction is ending, and this is the absolute + // last chance to perform any read operations in the current transaction. + // It is not recommended that the listener perform any writes. + TRANSACTION_ENDING, // Means the WriteTransaction has been released and the listener // can now take action on the changes it calculated. TRANSACTION_COMPLETE, @@ -594,7 +598,7 @@ struct DirectoryChangeEvent { } todo; // These members are only valid for CALCULATE_CHANGES. const OriginalEntries* originals; - BaseTransaction* trans; + BaseTransaction* trans; // This is valid also for TRANSACTION_ENDING WriterTag writer; typedef DirectoryChangeEvent EventType; static inline bool IsChannelShutdownEvent(const EventType& e) { @@ -1044,6 +1048,8 @@ class BaseTransaction { inline Directory* directory() const { return directory_; } inline Id root_id() const { return Id(); } + virtual ~BaseTransaction(); + protected: BaseTransaction(Directory* directory, const char* name, const char* source_file, int line, WriterTag writer); @@ -1072,7 +1078,7 @@ class ReadTransaction : public BaseTransaction { ReadTransaction(const ScopedDirLookup& scoped_dir, const char* source_file, int line); - ~ReadTransaction(); + virtual ~ReadTransaction(); protected: // Don't allow creation on heap, except by sync API wrapper. friend class sync_api::ReadTransaction; |