Uploaded image for project: 'Couchbase Lite'
  1. Couchbase Lite
  2. CBL-2658

Revise c4socket close shortcut logic

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • Major
    • Deuterium
    • 3.0, 2.8.4
    • LiteCore
    • Security Level: Public
    • None
    • Jianmin 66
    • 5

    Description

      The problem has been described in the CBSE-11072:

      Pasin:

      From the C4SocketFactory implemented by CBLWebSocket's point of view, here is what is happening:

      1. open() is called to open the socket and connect to the remote server.
      2. close() is called for closing the socket / connection as the replicator is stopped in a process of closing the database.
      3. dispose() is called to disposed the web socket. Noticed that the platform hasn't called either c4socket_opened() as it's not done connecting or c4socket_closed() to acknowledge the close complete yet.
      4. Replicator is stopped and database is closed.
      5. The test crashed as it is trying when trying to get cookies from the closed database. 

      Based on https://github.com/couchbase/couchbase-lite-core/blob/master/C/include/c4SocketTypes.h#L113-L120 , c4socket_closed() should be acknowledged after the close() is called (before the replicator can be stopped and before the C4SocketFactory's dispose() is called).

      Pasin:

      I have seen the comment in WebSocketImpl here regarding the shortcut:
      https://github.com/couchbase/couchbase-lite-core/blob/release/hydrogen/Networking/WebSockets/WebSocketImpl.cc#L373-L382

      Even though we are doing the shortcut but close() is called to the platform's C4SocketFactory, should it wait for c4socket_closed() before completing the close?

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              jianmin.zhao Jianmin Zhao
              Jayahari.Vavachan Jay Vavachan
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty