Skip to content

Conversation

hackjutsu
Copy link

  1. In the justlaunch scenario, the python script should move forward only if it launches the app successfully. If it fails, it should propagate the exit code out to notify the the users to handle the failure (perhaps relaunch?), instead of being trapped in the following loop in the safequit method.
  2. The changes aim to solve the hanging issue #189 of ios-deploy --justlaunch --debug --bundle my.app.
  3. I only tested the changes in the -justlaunch scenario.

1. the python script should move forward only if it launches the app
successfully
@omefire
Copy link

omefire commented Apr 4, 2016

I've tested this change with 'cordova', and running 'cordova run ios' still results in the process hanging on this line:

(lldb)     command script add -s asynchronous -f fruitstrap_3730c9a14ec0f5fb666eaa1f758ddd685dee2d55.safequit_command safequit
(lldb)     connect
(lldb)     run
success
(lldb)     safequit

@akofman
Copy link

akofman commented Apr 4, 2016

Same here using XCode 7.3 and ios-deploy with your fix.

@hackjutsu
Copy link
Author

@omefire @akofman Thanks for the feedback:)

  1. How often does it happen? I tested with Xcode 7.2 with 500 continuous launches. Maybe that is not enough...
  2. Would it be a good idea to add a timeout for the safequit method? If the ios-deploy fails to launch the app, it should at least return an error code after a timeout rather than just hanging.

Update:
I'm surprised that the hang now happens for every launch with Xcode 7.3, it only happened once or twice for every 100 launches before. I might need some actual testing with the new Xcode before I can reach any conclustion.

@akofman
Copy link

akofman commented Apr 4, 2016

I just tested it manually and it happens every times :/

@omefire
Copy link

omefire commented Apr 4, 2016

Same here, manual test and I always face this issue.

@shazron
Copy link

shazron commented Apr 5, 2016

Might be an Xcode 7.3 thing.

@akofman
Copy link

akofman commented Apr 5, 2016

@shazron : if it can help, XCode 7.2 is using lldb-340.4.119.1 while XCode 7.3 is using lldb-350.0.21.3.

@akofman
Copy link

akofman commented Apr 5, 2016

I tried a fix but not sure it is fine for every version of XCode : akofman@396daae.
Could you have a try guys ?

@shazron
Copy link

shazron commented Apr 5, 2016

I'll check it out @akofman

@shazron
Copy link

shazron commented Apr 5, 2016

@akofman good news, and bad news -- it solves the hanging issue, but now the app quits after launch.

@akofman
Copy link

akofman commented Apr 5, 2016

Ah ? I'll try on ios9.3 and XCode 7.3 and the app stayed alive.

@shazron
Copy link

shazron commented Apr 5, 2016

I'm using Xcode 7.3, iOS 9.3.1 (iPhone 6s), and launching using "cordova run --device"

@akofman
Copy link

akofman commented Apr 5, 2016

Ah yes sorry, I was testing on ios8. With ios9.3, indeed the app quits. grrrr

@akofman
Copy link

akofman commented Apr 5, 2016

I noticed the app doesn't quit actually but goes in background.

@shazron
Copy link

shazron commented Apr 5, 2016

So when I commented out this line:
https://github.com/phonegap/ios-deploy/blob/77654faabd890103fad5beea9cc7a80260f00f21/src/scripts/lldb.py#L56

After a wait, I now get:

(lldb)     safequit
Process 4113 detached
Error: Error code 9 for command: ios-deploy with args: --justlaunch,--no-wifi,-d,-b,/Users/shaz/Desktop/f/platforms/ios/build/device/f.app

@shazron
Copy link

shazron commented Apr 5, 2016

Now I can't repro the hanging state anymore -- very strange.

@akofman
Copy link

akofman commented Apr 5, 2016

Even with this line commented, it still hangs on my side :(

@shazron
Copy link

shazron commented Apr 5, 2016

I've rebooted my computer, etc, and still can't get this issue to repro, when I could before. I did have a fresh install of Xcode 7.3 hours ago, not sure what changed in the interim (did it install something in the background?)

@akofman
Copy link

akofman commented Apr 5, 2016

Without any changes in the code ?

@shazron
Copy link

shazron commented Apr 5, 2016

Yes, no changes.

@akofman
Copy link

akofman commented Apr 5, 2016

And you're still using an ios 9.3 ?? WTH

@shazron
Copy link

shazron commented Apr 5, 2016

I see in the Xcode 7.3 Release Notes: "Debugging can hang on launch if an .lldbinit file is present that tries to call quit in Python code during debugger initialization. (24360903)"
-- we don't use an .lldbinit file but we do send commands through lldb -s [lldb_command_file] which is is similar to lldbinit, but our initialization code doesn't call quit however

@shazron
Copy link

shazron commented Apr 5, 2016

@akofman Nothing has changed. I will have to try tonight on another computer. The code is the same, so it has to be my "environment" somehow.

@akofman
Copy link

akofman commented Apr 5, 2016

Did you check if your lldb version has been changed ? XCode 7.3 (7D175) => lldb-350.0.21.3

@shazron
Copy link

shazron commented Apr 5, 2016

No change, it's XCode 7.3 (7D175) => lldb-350.0.21.3

@hackjutsu
Copy link
Author

Guys, I wish I could help but I am stuck with a python error ImportError: No module named six when running ios-deploy with Xcode 7.3.

@shazron
Copy link

shazron commented Apr 5, 2016

@hackjustu that's strange, ios-deploy does not import any module named "six" at all, I believe everything is standard lib

@shazron
Copy link

shazron commented Apr 5, 2016

@hackjustu Seems it's one of the lldb python init files
https://bugs.swift.org/browse/SR-1061 which contains this link http://hastebin.com/obihaxujep.rb

@omefire
Copy link

omefire commented Apr 6, 2016

@shazron Trying it out ...

@shazron
Copy link

shazron commented Apr 6, 2016

Update: I didn't have time to write my results before I left, but I did more testing after my last statement, and I could eventually repro if I ran the command immediately again after a successful quit. It's sporadic. When running, ios-deploy should have two processes, plus one lldb process. "killall ios-deploy" and "killall lldb" to have a clean slate.

The essential problem is, lldb is not exiting, thus ios-deploy cannot exit as well.

@akofman
Copy link

akofman commented Apr 6, 2016

It's really weird you're not able to reproduce it like before. In my case, it hangs every time we try to detach the app from the debugger.
Watching this repo I noticed that this Detach method try to get a mutex, and I think the mutex is already locked.

As an example If I add a log after this line, it will never be printed.

What do you think ?

@hackjutsu
Copy link
Author

@akofman Did you actually get eStateRunning? I met a situation when state was always eStateLaunching and the process got stuck in the while loop. (Xcode 7.2 + iOS 8)

@shazron
Copy link

shazron commented Apr 6, 2016

@akofman doing a blame those lines are from years ago, and would probably be in Xcode 7.2. I'm trying to look at commits from 2015 https://github.com/llvm-mirror/lldb/commits/master/source/API/SBProcess.cpp

@shazron
Copy link

shazron commented Apr 6, 2016

trying to find changes since lldb-340.4.119.1 - not sure how that is tagged

@shazron
Copy link

shazron commented Apr 6, 2016

I believe I found the culprit. I'm on a different computer and I can repro reliably. It's not ios-deploy or lldb, it appears to be how cordova is calling ios-deploy. See if you can repro my results.

Make sure you have the latest ios-deploy and cordova and Xcode 7.3:

  1. npm install -g ios-deploy
  2. npm install -g cordova

Create a new Cordova project with the iOS platform:

  1. cordova create f f f
  2. cd f
  3. cordova platform add ios
  4. cordova build
  5. cordova run --device <-- this will hang
  6. ios-deploy -d -b platforms/ios/build/device/f.app --justlaunch <-- this will not hang

I've tried it a lot of times, with the results above. Make sure you do a killall ios-deploy && killall lldb between the alternate runs.

Perhaps cordova's usage of nodejs' child_process may be the culprit:

  1. https://github.com/apache/cordova-ios/blob/6cc86efbb17f59f2a9626b0867406be697fd47ab/bin/templates/scripts/cordova/lib/run.js#L135-L139
  2. https://github.com/apache/cordova-ios/blob/master/bin/templates/scripts/cordova/lib/spawn.js

@akofman
Copy link

akofman commented Apr 7, 2016

@hackjustu: yep I get the eStateRunning. If I add a log before process.Detach(), it is well printed while the log after is never.

@shazron :
It doesn't change anything on my side :(
ios-deploy -d -b platforms/ios/build/device/f.app --justlaunch always hangs.

I just noticed you're using ios 9.3.1 and I'm using ios 9.3. I'm going to update my iPad and check again.

@akofman
Copy link

akofman commented Apr 7, 2016

It is the same using ios 9.3.1.
An other point is that I'm testing on an iPad mini A1432.

@akofman
Copy link

akofman commented Apr 7, 2016

If I add a quite long sleep before detaching the process, it works. But it needs like a sleep of 20s.
And at least I can see the log indicating the process is well detached :

(lldb) safequit
Process 238 detached

@akofman
Copy link

akofman commented Apr 7, 2016

Ok. So I investigated a little bit more. It looks like without the waitForEvent method, everything works well and detaching the process doesn't hang anymore.
Regarding this part, I don't really understand why we are trying to retrieve the state from the event.
@shazron ? Maybe it is a legacy fix for older usecases, I never managed to go through this part of the code.

So I just removed all this part and simply get the state of the process from ... the process himself => akofman@4400067

I tested it on ios 9.3, ios 9.3.2 and ios 8.1 using ios-deploy -d -b App.app --justlaunch
and cordova run ios --device. it works well and quickly for both.

@omefire: could you have a try ?

@shazron
Copy link

shazron commented Apr 7, 2016

@akofman I'm not sure. The commit is here perhaps @senthilmanick can comment since I merged it in from his implementation.

@akofman
Copy link

akofman commented Apr 7, 2016

Yep, would be nice to have an explanation from the commiter :)
Else did you test it from your device ?

@shazron
Copy link

shazron commented Apr 9, 2016

Is this superseded by #223 now? :)

@akofman
Copy link

akofman commented Apr 11, 2016

I think yes. @hackjustu agree ?

@hackjutsu
Copy link
Author

Yes, even the run_command fails, it won't hang now thanks to #223.

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.

4 participants