So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`. I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following: # duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork` # add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync. Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable? Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient. Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic updates at least. This is problematic as well, but this is not used in regular release builds.
Bug 1687001 Comment 4 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`. I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following: 1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork` 2. add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync. Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable? Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient. Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic updates at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`. I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following: 1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork` 2. add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync. Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable? Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient. Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`. I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following: 1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork` 2. add a mutex protecting the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync. Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable? Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient. Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`. I think it's hard to avoid that we call `OpenDatabaseOp::BeginVersionChange` on the background thread. We could do either of the following: 1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork` 2. add a mutex protecting the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync. Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable? Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient. Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.