Skip to content

Conversation

@ehsannas
Copy link
Contributor

See similar logic in Android and Web.

This is inserting into a map while its being iterated over. Note that SortedMap
does not guarantee pointer stability. In fact, upon insertion, SortedMap may
switch to using a different underlying data structure (tree rather than array)
which invalidates pointers.

Fixes #9965.

@google-oss-bot
Copy link

Size Report 1

Affected Products

  • FirebaseFirestore

    TypeBase (c105a0d)Merge (eabd7bf)Diff
    CocoaPods?6.94 MB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/4dNSTngAXZ.html

@google-oss-bot
Copy link

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from ? (c105a0d) to 88.58% (eabd7bf) by ?.

    212 individual files with coverage change

    FilenameBase (c105a0d)Merge (eabd7bf)Diff
    any.nanopb.cc?0.00%?
    array_contains_any_filter.cc?100.00%?
    array_contains_filter.cc?100.00%?
    async_queue.cc?100.00%?
    auth_token.cc?100.00%?
    autoid.cc?100.00%?
    background_queue.cc?100.00%?
    bits.cc?100.00%?
    bound.cc?84.29%?
    bundle.nanopb.cc?0.00%?
    bundle_loader.cc?95.65%?
    bundle_reader.cc?91.26%?
    bundle_serializer.cc?90.92%?
    byte_stream_apple.mm?86.36%?
    byte_stream_cpp.cc?83.33%?
    byte_string.cc?80.30%?
    collection_reference.cc?100.00%?
    common.nanopb.cc?34.52%?
    comparison.cc?100.00%?
    connectivity_monitor.cc?100.00%?
    connectivity_monitor_apple.mm?51.92%?
    converters.mm?100.00%?
    database_id.cc?65.38%?
    database_info.cc?100.00%?
    datastore.cc?97.06%?
    delete_mutation.cc?84.00%?
    direction.cc?76.92%?
    document.cc?0.00%?
    document.nanopb.cc?96.52%?
    document_change.cc?62.50%?
    document_key.cc?93.06%?
    document_key_reference.cc?65.00%?
    document_overlay_cache.cc?100.00%?
    document_reference.cc?100.00%?
    document_set.cc?87.30%?
    document_snapshot.cc?100.00%?
    empty.nanopb.cc?0.00%?
    error_apple.mm?92.31%?
    event_manager.cc?98.08%?
    exception.cc?84.21%?
    exception_apple.mm?65.52%?
    executor_libdispatch.mm?96.06%?
    executor_std.cc?97.71%?
    exponential_backoff.cc?100.00%?
    field_filter.cc?90.18%?
    field_index.cc?92.68%?
    field_mask.cc?100.00%?
    field_path.cc?98.17%?
    field_transform.cc?41.67%?
    filesystem_apple.mm?83.33%?
    filesystem_common.cc?81.67%?
    filesystem_posix.cc?81.82%?
    filter.cc?62.50%?
    FIRCollectionReference.mm?95.08%?
    FIRDocumentChange.mm?76.32%?
    FIRDocumentReference.mm?97.10%?
    FIRDocumentSnapshot.mm?97.89%?
    firebase_app_check_credentials_provider_apple.mm?92.93%?
    firebase_auth_credentials_provider_apple.mm?76.64%?
    firebase_metadata_provider_apple.mm?86.96%?
    firebase_metadata_provider_noop.cc?100.00%?
    firestore.cc?95.03%?
    firestore.nanopb.cc?39.15%?
    firestore_client.cc?98.81%?
    firestore_index_value_writer.cc?90.00%?
    FIRFieldPath.mm?90.24%?
    FIRFieldValue.mm?89.41%?
    FIRFilter.mm?100.00%?
    FIRFirestore.mm?91.43%?
    FIRFirestoreSettings.mm?85.53%?
    FIRFirestoreSource.mm?90.91%?
    FIRGeoPoint.mm?82.22%?
    FIRListenerRegistration.mm?100.00%?
    FIRLoadBundleTask.mm?80.70%?
    FIRQuery.mm?88.00%?
    FIRQuerySnapshot.mm?95.45%?
    FIRSnapshotMetadata.mm?100.00%?
    FIRTimestamp.m?79.35%?
    FIRTransaction.mm?97.64%?
    FIRTransactionOptions.mm?100.00%?
    FIRWriteBatch.mm?100.00%?
    FSTFirestoreComponent.mm?97.83%?
    FSTUserDataReader.mm?95.38%?
    FSTUserDataWriter.mm?87.06%?
    geo_point.cc?65.00%?
    grpc_completion.cc?100.00%?
    grpc_connection.cc?77.50%?
    grpc_nanopb.cc?94.87%?
    grpc_root_certificate_finder_generated.cc?100.00%?
    grpc_stream.cc?99.01%?
    grpc_streaming_reader.cc?100.00%?
    grpc_unary_call.cc?100.00%?
    grpc_util.cc?100.00%?
    hard_assert.cc?100.00%?
    http.nanopb.cc?0.00%?
    index.nanopb.cc?0.00%?
    index_backfiller.cc?100.00%?
    index_entry.cc?60.00%?
    in_filter.cc?100.00%?
    key_field_filter.cc?100.00%?
    key_field_in_filter.cc?100.00%?
    key_field_not_in_filter.cc?100.00%?
    latlng.nanopb.cc?84.62%?
    leveldb_bundle_cache.cc?76.00%?
    leveldb_document_overlay_cache.cc?97.17%?
    leveldb_index_manager.cc?96.17%?
    leveldb_key.cc?98.33%?
    leveldb_lru_reference_delegate.cc?94.31%?
    leveldb_migrations.cc?92.64%?
    leveldb_mutation_queue.cc?92.42%?
    leveldb_opener.cc?76.81%?
    leveldb_overlay_migration_manager.cc?100.00%?
    leveldb_persistence.cc?90.82%?
    leveldb_remote_document_cache.cc?94.38%?
    leveldb_target_cache.cc?94.68%?
    leveldb_transaction.cc?98.79%?
    leveldb_util.cc?71.43%?
    load_bundle_task.cc?97.06%?
    local_documents_view.cc?96.82%?
    local_serializer.cc?87.19%?
    local_store.cc?100.00%?
    local_view_changes.cc?100.00%?
    log_apple.mm?93.33%?
    lru_garbage_collector.cc?91.34%?
    maybe_document.nanopb.cc?30.95%?
    memory_bundle_cache.cc?100.00%?
    memory_document_overlay_cache.cc?100.00%?
    memory_eager_reference_delegate.cc?100.00%?
    memory_index_manager.cc?48.48%?
    memory_lru_reference_delegate.cc?95.41%?
    memory_mutation_queue.cc?98.78%?
    memory_persistence.cc?100.00%?
    memory_remote_document_cache.cc?93.06%?
    memory_target_cache.cc?100.00%?
    message.cc?100.00%?
    mutable_document.cc?68.52%?
    mutation.cc?86.15%?
    mutation.nanopb.cc?77.42%?
    mutation_batch.cc?88.30%?
    mutation_batch_result.cc?52.17%?
    nanopb_util.cc?100.00%?
    not_in_filter.cc?100.00%?
    object_value.cc?100.00%?
    online_state_tracker.cc?100.00%?
    ordered_code.cc?93.90%?
    order_by.cc?50.00%?
    overlay.cc?100.00%?
    patch_mutation.cc?100.00%?
    path.cc?100.00%?
    precondition.cc?86.49%?
    pretty_printing.cc?83.33%?
    proto_sizer.cc?72.73%?
    query.cc?98.39%?
    query.nanopb.cc?87.85%?
    query_core.cc?96.20%?
    query_engine.cc?98.28%?
    query_listener.cc?100.00%?
    query_listener_registration.cc?100.00%?
    query_snapshot.cc?84.21%?
    reader.cc?100.00%?
    reference_set.cc?88.89%?
    remote_event.cc?97.10%?
    remote_objc_bridge.cc?90.23%?
    remote_store.cc?90.64%?
    resource.nanopb.cc?0.00%?
    resource_path.cc?100.00%?
    schedule.cc?100.00%?
    secure_random_arc4random.cc?100.00%?
    serializer.cc?89.91%?
    server_timestamp_util.cc?97.06%?
    settings.cc?0.00%?
    set_mutation.cc?89.13%?
    snapshots_in_sync_listener_registration.cc?100.00%?
    snapshot_metadata.cc?100.00%?
    snapshot_version.cc?85.71%?
    status.cc?73.05%?
    status.nanopb.cc?75.00%?
    statusor.cc?80.00%?
    status_apple.mm?96.61%?
    status_errno.cc?30.25%?
    stream.cc?98.83%?
    strerror.cc?100.00%?
    string_apple.cc?100.00%?
    string_format.cc?93.94%?
    string_util.cc?100.00%?
    struct.nanopb.cc?7.59%?
    sync_engine.cc?95.20%?
    target.cc?95.77%?
    target.nanopb.cc?45.95%?
    target_data.cc?40.63%?
    target_id_generator.cc?100.00%?
    target_index_matcher.cc?97.65%?
    task.cc?94.78%?
    timestamp.cc?94.00%?
    timestamp.nanopb.cc?100.00%?
    timestamp_internal.cc?53.33%?
    transaction.cc?89.81%?
    transaction_runner.cc?100.00%?
    transform_operation.cc?77.03%?
    user.cc?100.00%?
    user_data.cc?96.30%?
    value_util.cc?95.71%?
    verify_mutation.cc?12.50%?
    view.cc?98.37%?
    view_snapshot.cc?78.63%?
    watch_change.cc?90.00%?
    watch_stream.cc?90.70%?
    wrappers.nanopb.cc?9.26%?
    write.nanopb.cc?62.11%?
    writer.cc?96.97%?
    write_batch.cc?91.89%?
    write_stream.cc?91.55%?

  • FirebaseFirestore-iOS-FirebaseFirestoreSwift.framework

    Overall coverage changed from ? (c105a0d) to 48.21% (eabd7bf) by ?.

    21 individual files with coverage change

    FilenameBase (c105a0d)Merge (eabd7bf)Diff
    CodablePassThroughTypes.swift?100.00%?
    CollectionReference+AsyncAwait.swift?96.88%?
    CollectionReference+WriteEncodable.swift?100.00%?
    DocumentID.swift?80.00%?
    DocumentReference+Codable.swift?50.00%?
    DocumentReference+ReadDecodable.swift?0.00%?
    DocumentReference+WriteEncodable.swift?100.00%?
    DocumentSnapshot+ReadDecodable.swift?80.00%?
    ExplicitNull.swift?90.48%?
    FieldValue+Encodable.swift?100.00%?
    Firestore+AsyncAwait.swift?97.75%?
    FirestoreDecoder.swift?48.74%?
    FirestoreEncoder.swift?61.78%?
    FirestoreQuery.swift?0.00%?
    FirestoreQueryObservable.swift?0.00%?
    GeoPoint+Codable.swift?100.00%?
    QueryPredicate.swift?0.00%?
    ServerTimestamp.swift?96.97%?
    Timestamp+Codable.swift?100.00%?
    Transaction+WriteEncodable.swift?100.00%?
    WriteBatch+WriteEncodable.swift?100.00%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/s93EG2I7Gp.html

@ehsannas ehsannas requested a review from dconeybe August 13, 2022 03:57
@ehsannas ehsannas requested a review from wu-hui August 13, 2022 03:59
Copy link
Contributor

@dconeybe dconeybe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'll defer to @wu-hui who understands the code better.

@wu-hui wu-hui assigned ehsannas and unassigned wu-hui Aug 15, 2022
@ehsannas ehsannas merged commit 5368508 into master Aug 15, 2022
@ehsannas ehsannas deleted the ehsann/fix-9965 branch August 15, 2022 14:40
@firebase firebase locked and limited conversation to collaborators Sep 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App crashed on data insert & delete in firestore

4 participants