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

Swift Client: Fix double deallocation #3668

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weipin
Copy link

@weipin weipin commented Jun 27, 2021

Hello,

This PR is an attempt to fix a double deallocation issue in the Swift client.

How to produce

  1. In the Swift demo project deepspeech_ios_test, locate the file SpeechRecognitionImpl.swift, and replace let result = stream.finishStream() with let result = stream.finishStreamWithMetadata(numResults: 1)
  2. Run the app and click the button "Recognize files".
  3. The app will crash as soon as the associated DeepSpeechStream is deallocated. In my case, Xcode prompted something like "Thread XXX: Deallocation of freed memory".

References

Fix

Seems the fix can be as simple as adding one line of code. After the patch, the code pattern of finishStreamWithMetadata(numResults:) now aligns with finishStream().

Thank you

Would like to thank @reuben for the Swift wrapper-related work!

@JanX2
Copy link
Contributor

JanX2 commented Aug 14, 2021

Could you please consider merging this, @reuben? Thanks!

@reuben
Copy link
Contributor

reuben commented Aug 14, 2021

I no longer have push access to this repository. If you can open the PR against our coqui-ai/STT fork I'm happy to merge it.

@weipin
Copy link
Author

weipin commented Aug 15, 2021

Few weeks ago I was planning to do that (opening a PR against coqui). But failed to build the iOS static framework -- bazel prompts:

"ERROR: file 'native_client/libkenlm.so' is generated by these conflicting actions:".

Will give it another try later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants