johnno1962 / InjectionIII

Re-write of Injection for Xcode in (mostly) Swift
MIT License
4.08k stars 319 forks source link

Crash in sweepMembers function (avoid unowned ivars) #68

Closed skrew closed 4 years ago

skrew commented 6 years ago

Hi,

I got EXEC_BAD_ACCESS at for (_, value) in mirror!.children { in function sweepMembers(_ instance: Any)

It crash when mirror is one of my project Class...

For testing, i have filtered out theses classes, and injection works.

while mirror != nil && mirror?.description != "Mirror for ViewPlayerController" && mirror?.description != "Mirror for VPSettingsSubtitle" && mirror?.description != "Mirror for VPSettingsVideo" && mirror?.description != "Mirror for VPSettingsAudio" {

Note: In my test, i make injection in VPSettingsSubtitle class. Even with this class filtered, injection works.

zenangst commented 6 years ago

Interesting, and this happens consistently with those classes? Mind sharing some additional information/characteristics about the classes so that we can narrow it down a bit?

skrew commented 6 years ago

It's just "normal" classes (UIViewController / UITableViewController). Sure there is something wrong with the project, but it's a big one, it's difficult to identify the problem...

Maybe the only common pattern: all are "presented" (not pushed in a UINavigationController)

zenangst commented 6 years ago

Interesting, I would love some additional logs to dig into this further. Do you get any additional information if you enable exception breakpoints? Maybe you could trigger the error again and have a look at the last argument when it does crash. For more information, please have a look at https://www.natashatherobot.com/xcode-debugging-trick/

skrew commented 6 years ago

This is all i can get right now. At the top of the crash stack, po $arg1 (or more) don't give anything:

error: Couldn't materialize: couldn't read the value of register rdi
error: errored out in DoExecute, couldn't PrepareToExecuteJITExpression
(lldb) po mirror!
Mirror for ViewPlayerController
(lldb) po mirror!.children
▿ AnyCollection<(label: Optional<String>, value: Any)>
  ▿ _box : <_RandomAccessCollectionBox<LazyMapCollection<Range<Int>, (label: Optional<String>, value: Any)>>: 0x600000e06bc0>
screen shot 2018-11-14 at 18 42 44
zenangst commented 5 years ago

@skrew hey mate, you think you could make a small example project for me where this is reproducible?

skrew commented 5 years ago

It will be hard, but i can open a team viewer session on this mac if it can help you

zenangst commented 5 years ago

Hey @skrew, I've done some investigation here as I ran into a similar issue. Mind trying out this new version to see if it works better for you?

https://www.dropbox.com/s/7stoecnf6uubno1/InjectionIII%20-%201.4%20Preview%202.zip?dl=0

skrew commented 5 years ago

Thanks, i have tested it but i still got a crash (look like in the same function)

screen shot 2018-11-26 at 16 52 39
zenangst commented 5 years ago

@skrew Thanks for testing mate!

If you highlight the 16 step in the stack trace, SwiftSweeper.sweepInstance(_:), what type is the instance that it keeps crashing on?

skrew commented 5 years ago

i can't get any information about the crash in this executable (compiled with debug ?) I can just get registers (but not very useful)...

General Purpose Registers:
       rbx = 0x00006000007d5240
       rbp = 0x00007ffeeea62570
       rsp = 0x00007ffeeea623b0
       r12 = 0x0000000000000000
       r13 = 0x0000600008fcb120
       r14 = 0x000000012b1608c0  iOSInjection10`__34-[InjectionClient runInBackground]_block_invoke.1291
       r15 = 0x000000010d81b589  libdispatch.dylib`_dispatch_call_block_and_release
       rip = 0x000000012b1afd0a  iOSInjection10`iOSInjection10.SwiftSweeper.sweepInstance(Swift.AnyObject) -> () + 858
zenangst commented 5 years ago

@skrew would you mind downloading the latest version and enabling DEBUG_SWEEP? Then we could get some more information about which class is causing the sweep method to crash. You enable it by setting the DEBUG_SWEEP environment in your application under targets.

skrew commented 5 years ago

Hi, Sorry for the delay... I have updated to the last version (head) and tested again, still crashing. Enabled DEBUG_SWEEP

LOG

Again, by adding while mirror != nil && mirror?.description != "Mirror for GSSettings" (...) avoid the crash and the injection in GSSettings works (injected()) is called just fine)

johnno1962 commented 5 years ago

Just thought I’d check but this project is ARC right?

skrew commented 5 years ago

@johnno1962 yep In Sept 2015 you have used teamviewer for debugging injection4xcode on one of my project, if you or @zenangst want to do it again, just tell.

johnno1962 commented 5 years ago

This sort of problem is going to be very difficult to track down. If possible I’d recommend using the INJECTION_BUNDLE_NOTIFICATION instead of -injected method for your app.

johnno1962 commented 5 years ago

Which version of Xcode is this?

skrew commented 5 years ago

Version 10.1 (10B61)

johnno1962 commented 5 years ago

So you have time for a teamview at the moment? If so email your details through to injection at johnholdsworth.com

skrew commented 5 years ago

Mail sent.

johnno1962 commented 5 years ago

In the end the problem was use of unowned ivars which do not mix well with Swift’s Mirror that Injection uses for the sweep. If I can replicate with a small example app I’ll file a radar though I’m not sure what the solution would be.

skrew commented 5 years ago

At least problem is fixed for me :) Thanks again @johnno1962 You can close it if it's ok for you

johnno1962 commented 5 years ago

I’ll leave it open as it is still very much a current issue. I’ve not been able to replicate it as yet.

johnno1962 commented 5 years ago

Replicated and radar’d you can not inject projects with unowned ivars that may have been dealloc’d. Thanks for persevering on this issue!

skrew commented 5 years ago

Just for info, changing unowned to weak is working (normal, it's optional...)

johnno1962 commented 5 years ago

Seems to be a known problem https://bugs.swift.org/browse/SR-5289

skrew commented 5 years ago

This is a bug in loadSpecialReferenceStorage() in swift_ClassMirror_subscript(). It assumes that any variable that is not weak is strong. This is incorrect for unowned variables. The crash occurs because an unowned variable pointing to an ObjC object does not simply store a pointer to the object.

Right, but it's identified since jun 2017... The 2 unowned vars you had see in the ViewPlayerController was:

unowned let ud: UserDefaults = UserDefaults.standard
unowned let mpvHandler: MPVHandler = MPVHandler.shared

MPVHandler is shared instance (singleton) and never get dealloc'ed (i don't think UserDefaults.standard can be released ?)

johnno1962 commented 5 years ago

https://twitter.com/Catfish_Man/status/1098788529360928769

wMellon commented 5 years ago

This sort of problem is going to be very difficult to track down. If possible I’d recommend using the INJECTION_BUNDLE_NOTICATION instead of -injected method for your app.

Name is wrong. The right name is INJECTION_BUNDLE_NOTIFICATION