Skip to content
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

Randomly crash after upgrade version from 0.0.6 to 1.0.3 on DictionaryStorageProvider.value. #78

Closed
loongman opened this issue Nov 29, 2023 · 7 comments · Fixed by #79
Closed
Labels
bug Something isn't working

Comments

@loongman
Copy link

Describe the bug

According to the crash reports, seems the crash is happen when we use UnleashClientBase.isEnabled or UnleashClientBase.getVariant right after we get the "ready" subscription callback.

public func subscribe(name: String, callback: @escaping () -> Void)

Btw, there is guarantee on our implementation to always use Unleash APIs on main thread.

Steps to reproduce the bug

The reproduce rate is not high, actually, we do not manage to reproduce it by ourselves. However, we are receiving crash reports intermittently after we release our product yesterday.

Expected behavior

No crash.

Logs, error output, etc.

No response

Screenshots

crash_report

Additional context

No response

Unleash version

1.0.3

Subscription type

None

Hosting type

None

SDK information (language and version)

No response

@loongman loongman added the bug Something isn't working label Nov 29, 2023
@FredrikOseberg
Copy link
Contributor

@loongman Thanks for reporting this, I'll try to set up a reproduction case. If you have any more informations about how we can reproduce this, please do share them here. Thank you!

@loongman
Copy link
Author

loongman commented Dec 1, 2023

Thanks. @FredrikOseberg

As mentioned on the issue description, according to the crash reports, seems the crash is happen when we use UnleashClientBase.isEnabled or UnleashClientBase.getVariant right after we get the "ready" subscription callback.

Below is the screenshot which provides an overview about the reports we received so far.

Screenshot 2023-12-01 at 14 05 40

We are now plan to make a small change(see below code snippet) and release it as a patch to see if it helps. If not, we has to rollback to v0.0.6 before you guys figured out the root cause of the issue.

Screenshot 2023-12-01 at 14 11 36

@FredrikOseberg
Copy link
Contributor

FredrikOseberg commented Dec 1, 2023

Thanks. @FredrikOseberg

As mentioned on the issue description, according to the crash reports, seems the crash is happen when we use UnleashClientBase.isEnabled or UnleashClientBase.getVariant right after we get the "ready" subscription callback.

Below is the screenshot which provides an overview about the reports we received so far.

Screenshot 2023-12-01 at 14 05 40 We are now plan to make a small change(see below code snippet) and release it as a patch to see if it helps. If not, we has to rollback to v0.0.6 before you guys figured out the root cause of the issue. Screenshot 2023-12-01 at 14 11 36

@loongman According to my findings yesterday, this seems to be a concurrency issue that occurs when the SDK has fetched feature toggles from the API and is calling the createFeatureMap function that puts the data into the dictionary at the exact same time as a thread is trying to read from it. I'm looking into possible solutions and should have a fix out this morning. What worked for me yesterday was wrapping the DictionaryProvider with nslock:

public class DictionaryStorageProvider: StorageProvider {
    private var storage: [String: Toggle] = [:]
    private let lock = NSLock()

    public init() {}

    public func set(value: Toggle?, key: String) {
        lock.lock()
        storage[key] = value
        lock.unlock()
    }

    public func value(key: String) -> Toggle? {
        lock.lock()
        let value = storage[key]
        lock.unlock()
        return value
    }
    
    public func clear() {
        lock.lock()
        storage = [:]
        lock.unlock()
    }
}

It's possible in the SDK to input your own poller implementation and thus pass the poller your own storageprovider, so if you don't have time for me to get the fix out this morning that could be an option. I'm currently evaluating if this is the best approach or if I should just create a new dict when the SDK receives new data and replace the current one after I put all the data into it. This will be my top priority until the fix is out. I'll keep you posted.

@FredrikOseberg
Copy link
Contributor

@loongman I've released a new version that should resolve this issue. Can you try version 1.1.1?

@loongman
Copy link
Author

loongman commented Dec 4, 2023

@FredrikOseberg Thanks a lot for the fix you have made.

We have integrated version 1.1.1 and released a patch. We need couple of days to observe the results.

@loongman
Copy link
Author

loongman commented Dec 5, 2023

There is no crash report received till now on our patch with Unleash SDK v1.1.1. In other words, your fix works as expected. Thanks! @FredrikOseberg

@loongman loongman closed this as completed Dec 5, 2023
@github-project-automation github-project-automation bot moved this from New to Done in Issues and PRs Dec 5, 2023
@FredrikOseberg
Copy link
Contributor

@loongman Fantastic. I'm glad it's resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants