-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Recent master commit causes crash with ASEditableTextNode inside an ASCellNode #607
Comments
Could you please update the symbolized trace? |
Yeap, I’ll take a closer look. Thanks for reporting! |
@nguyenhuy I have noticed that when the crash happens on assert the current thread is This is the log:
|
@hebertialmeida Hm, that shouldn't be happening. Based on the log, the text components should be added to the main thread queue. I'm suspecting there is a race condition somewhere. Can you reproduce this problem every time? When it happens, at which line inside |
No, It doesn't happen every time, sometimes after couple tries. I have created a sample project that reproduces, there is also a test case that you can execute and will open and dismiss 100x so you don't have to click on that, just execute the test and I hope it crashes. Seems that when the crash happens it is passing on line
|
@nguyenhuy Did you have a chance to test it? |
Hey, looks like I'm having the same issue: on Here's the stack trace for the dealloc queue when the crash occurs (using commit
|
@flovouin Probably it is the same problem. |
This issue is happening for me too on 2.5.1 when I'm using |
Quick update: There is a race condition in |
@nguyenhuy Thanks! |
same to me, which version is safe for now? |
Please test against the diff in #651. |
Thanks @nguyenhuy, just tested that and It fixed to me. I think this can be closed... |
Awesome. Thank you! |
Sorry for the delay. It seems to fix the bug on my end too. Cheers! |
Any chance this could be added in next release? because version 2.5.1 is still crashing due to this issue. Thanks |
Yes it’ll be included in the next release. |
In the meantime, please use master if you have to. FYI, Pinterest syncs with master pretty much every week. |
I also use master without problem 😄 |
We are observing similar crash. It is not always reproducible.
|
I am working on the master branch and a recent
pod update
caused my app to start instant crashing at a new main thread assert in the dealloc method ofASTextKitComponents
, which I noticed was added recently in the commit hereAfter struggling to find what was causing it for ages, I eventually worked out that it simply seemed to be caused by having an
ASEditableTextNode
as a child of anASCellNode
in anASCollectionNode
. To confirm this I managed to retro-fit an example Texture project which I created a few weeks ago to demonstrate another issue. I have attached this here:TextureGifMemoryBug ASEditableTextNode Crash.zip
The project was originally created to demonstrate a memory issue using gifs. As such when you run the project you'll see an
ASCollectionNode
with a lot of gifs. To demonstrate the crash issue discussed above, all you have to do is go into theGifCellNode
class and unblock the line of code at the top oflayoutSpecThatFits:
that says:This will switch the cells to instead use a simple
ASEditableTextNode
created in theinit
method, and as you will see will instantly crash the app on the assert added in the commit noted above.@maicki maybe you can help?
The text was updated successfully, but these errors were encountered: