New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swift Replicator.addChangeListener crashes #1926

Closed
jayzhanghs opened this Issue Oct 9, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@jayzhanghs

jayzhanghs commented Oct 9, 2017

    let replicator = Replicator(config: replicatorConfig)
    replicator.addChangeListener { (replicatorChange) in
    }

In my project the above code crashes at addChangeListener even if I have only an empty block, with errors "fatal error: attempted to read an unowned reference but object 0x60c000129560 was already deallocated". Really suspect that the unowned self designator in the framework definition of addChangeListener may have caused this.

Commenting out this call would stop the crash, but I also lost capability to track the changes from replicator.


  • Version: 2.0DB016
  • Client OS: iOS 11
  • Server: Sync Server
  • Environment: Xcode 9, Swift 3.2
@pasin

This comment has been minimized.

Contributor

pasin commented Oct 9, 2017

@rajagp are you seeing the same thing is the app you are working on?

@rajagp

This comment has been minimized.

Contributor

rajagp commented Oct 9, 2017

I am not seeing any crashes. This is what I have.

       _pushPullRepl = Replicator.init(config: config)
_pushPullReplListener = _pushPullRepl?.addChangeListener({ [weak self] (change) in
            let s = change.status
            print("PushPull Replicator: \(s.progress.completed)/\(s.progress.total), error: \(String(describing: s.error)), activity = \(s.activity)")
            if s.progress.completed == s.progress.total {
                self?.postNotificationOnReplicationState(.idle)
            }
            else {
                self?.postNotificationOnReplicationState(s.activity)
            }
        })
        

@jayzhanghs : Can you change the scope of the replicator object that you are creating so it has class scope and it is not locally scoped to the function.
For instance,in my case fileprivate var _pushPullRepl:Replicator? is defined as a file private variable scoped to my class.

@jayzhanghs

This comment has been minimized.

jayzhanghs commented Oct 10, 2017

@rajagp Yes, changing replicator object as an instance variable did make it work!

In the sample given in CouchLite 2.0 the replicator was created as a local variable. Please update the document.

Thanks!

@rajagp

This comment has been minimized.

Contributor

rajagp commented Oct 10, 2017

@pasin

This comment has been minimized.

Contributor

pasin commented Oct 11, 2017

@jayzhanghs please make the instance variable for the replicator. In Objective-C version, the started replicator is cached inside the database object as well so that you don't need to make the instance variable. I had the changes for this but I would like to hold it off a bit as there is a bit more work needs to be done to take care the case when closing the database. To sum up, the fix will not be in the next DB release which is DB18.

@pasin pasin changed the title from Swift Replicator.addChangeListener crashes to 2.0: Swift Replicator.addChangeListener crashes Oct 11, 2017

@djpongh djpongh added this to the 2.0.0 milestone Oct 13, 2017

@djpongh djpongh added the backlog label Oct 13, 2017

@djpongh djpongh added the P1: high label Nov 17, 2017

@rajagp rajagp added P1: high and removed P1: high labels Dec 19, 2017

@djpongh djpongh modified the milestones: 2.0.0, 2.0 DB 22 Jan 18, 2018

pasin added a commit that referenced this issue Jan 25, 2018

Fix active replicators and queries in swift are not retained
In Objective-C and other platforms, active replicators and queries are retained in the database object. Hence doing the same for Swift.

#1926

@pasin pasin self-assigned this Jan 25, 2018

@pasin pasin closed this Jan 25, 2018

@pasin pasin removed the backlog label Jan 25, 2018

@pasin

This comment has been minimized.

Contributor

pasin commented May 21, 2018

Look like the fix for this issue is not enough for a non-continuous replicator. I'm reopening the issue.

@pasin pasin reopened this May 21, 2018

@pasin pasin modified the milestones: 2.0.0, 2.1.0 May 21, 2018

@pasin pasin added ffc bug labels May 21, 2018

@pasin

This comment has been minimized.

Contributor

pasin commented May 21, 2018

Reference: CBSE-5304

@djpongh djpongh added backlog known-issue and removed P1: high labels Jun 1, 2018

@djpongh djpongh added icebox and removed backlog labels Jul 9, 2018

@pasin

This comment has been minimized.

Contributor

pasin commented Jul 20, 2018

This issue is not trivial to fix as the notification could be posted later in a queue. The workaround would be retaining the replicator object in an instance variable.

pasin added a commit that referenced this issue Jul 31, 2018

Fix swift single-shot replicator crashed when notifying change listener
Removed unowned marker as we should retain the self instance.

#1926
@pasin

This comment has been minimized.

Contributor

pasin commented Jul 31, 2018

After reviewing this issue again, the fix was fairy simple. The fix will be available in 2.1.

@pasin pasin closed this Jul 31, 2018

@pasin pasin changed the title from 2.0: Swift Replicator.addChangeListener crashes to Swift Replicator.addChangeListener crashes Jul 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment