Adoption of ConversationAvatarView2 in many key locations. This isn't
exhaustive, but it's mostly there.
Some work left to be done around ConversationAvatarView2 data source
tweaks.
- Optionals are now SDSSwiftSerializable as long as their wrapped type
conforms
- Avatar file writes are now performed outside of a write transaction
- Various rebase cleanup
We're seeing crashes during this data migration step. We fail to init
the OWSUserProfile from the Row result returned from the SQLite fetch
request. It's likely we're indexing into an out-of-bounds index on Row.
(Row._checkIndex is inlined so we can't be sure)
GRDB caches the sqlite3_column_count during Row.init and uses that to
determine if a subscript access is out-of-bounds. From what I can tell
in an amateur reading of sqlite3.c, that just pulls the nResColumn from
the prepared statement. SQLite will determine the number of result
columns during statement prep, but the column count can change if the
table schema changes:
> If the database schema changes, instead of returning SQLITE_SCHEMA as
it always used to do, sqlite3_step() will automatically recompile the
SQL statement and try to run it again. As many as
SQLITE_MAX_SCHEMA_RETRY retries will occur before sqlite3_step() gives
up and returns an error.
We're nesting a deferred read transaction inside of our data migration's
immediate transaction. This read is using a different sqlite connection
that the connection performing our migrations. So its understanding of
the schema is stale and because it's in a deferred transaction it won't
figure this out until we call _step().
Once we call _step(), sqlite will realize that the table schema has
changed and recompile the prepared statement while opening the
deferred transaction. After this, the sqlite_column_count is correct.
But Row still has the incorrect value cached from before the schema was
resolved.
So by the time we try and init our OWSUserProfile, Row falsely asserts
that we're indexing beyond the column count by comparing against a stale
version of the column count.
The fix:
Rework the data migration to avoid nesting a deferred read transaction.
This is incorrect behavior. Now that our read is using our already open
write transaction with the same connection handle, it's understanding of
the schema is up-to-date during statement prep. So Row.init grabs the
current column count.
