Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-24034] iOS: New logging system breaks differential builds - simulator builds only

GitHub Issuen/a
TypeBug
PriorityCritical
StatusClosed
ResolutionFixed
Resolution Date2016-10-18T02:47:24.000+0000
Affected Version/sn/a
Fix Version/sRelease 6.0.0
ComponentsiOS, Tooling
Labelsn/a
ReporterChris Barber
AssigneeChris Barber
Created2016-10-13T23:33:25.000+0000
Updated2018-08-06T17:49:34.000+0000

Description

The iOS Simulator shares ports with the host machine. The iOS build will select a log server port based on the <log-server-port> in the tiapp.xml or based on the hash of the app id using the following logic:
parseInt(sha1(this.tiapp.id), 16) % 50000 + 10000
Because we only allow a possible 50,000 possibilities, the likelihood of a collision is high. In my tests, it's about 5%. The solution to the collisions is to just allow manual override by setting a <log-server-port> in the tiapp.xml. The next problem is what if the app id hash port resolves a port number used by another piece of software? Say you have MongoDB installed and it's listening on port 27017. It's entirely possible Titanium will pick 27017. The current iOS build logic tries to be smart. It starts a Node.js server listening on the selected port and if it can't bind to it, then that means some other app already has bound to the port. It will then select a random port. I knew at design time that random ports will break the differential build system, but figured it would only happen if there was a collision. Turns out that's not quite accurate. If you build an app for the sim and then rebuild our app while leaving the previous build running in the sim, the iOS build will detect the desired port is not available because the iOS app is currently listening on it. This causes the iOS build to randomly select a port. The port number gets written to the Xcode project. Since it will generate a new random port every build (as long as the sim and previous build is running), the iOS build detects the Xcode project is different and forces a full rebuild. We have 5 solutions: 1. Don't select a random port. Just use the port regardless if it's in use. Cons: high chance of port collisions and false positives, the iOS app will fail to bind to it and there will be no logs and no warning. 2. Don't select a random port. If the port is already in use, try connecting to it and see if it's the app we're trying to build and if so, then proceed with the build. If the port is being used by something such as a web server, it will be expecting data, but because the log server clients do not send data, we end up in a stalemate. We would need to add a timeout. Cons: the timeout will pause the build by ~1 second. 3. Allow random ports if the desired port is unavailable. Pass the GCC_PREPROCESSOR_DEFINITIONS="TI_LOG_SERVER_PORT=XXXX" directly into xcodebuild instead of baking it into the Xcode project. Cons: you will run in a loop where the build will alternate between the hashed port and a random one, we would need to force xcodebuild to re-compile using the new port value. 4. Write the port to a file (could be any file) and bundle it with the app. Avoids the Xcode project and xcodebuild. If the file does not exist or is empty, then logging is disabled. Cons: easy to accidentally ship with the app and enable logging in production. 5. Pass the port number in as an argument when we call simctl launch. We would still bake the DISABLE_TI_LOG_SERVER=1 into the Xcode project for dist builds. Cons: ioslib currently does not support passing arguments into simctl launch, subsequent manually launching the app will not have the argument. I think option 1 and 3 are not too good. Option 2 is not so great. Option 4 is not too bad. https://www.youtube.com/watch?v=vm-MrkoJPC8 Thoughts?

Attachments

FileDateSize
cleanBuild.trace2016-10-17T13:52:27.000+00001608787
subseqBuild.trace2016-10-17T13:52:25.000+000051092

Comments

  1. Hans Knöchel 2016-10-14

    I like option 2 if the timeout doesn't actively the build. When you say it could be done when resources are copied, it sounds like a good idea. I also like option 4, if we can ensure that it doesn't get shipped with production apps. Could be store it in the build dir or does that also make problems with recurring builds?
  2. Chris Barber 2016-10-14

    Turns out we can't resolve log server port while copying resources... we must do it before we write the Xcode project which is right before we do all the parallel tasks.
  3. Chris Barber 2016-10-14

    I did a bunch of testing and ended up going with Option 2. I removed the code that selects a random port. New logic:

    Select the port to use based on either the <log-server-port> in the tiapp.xml or generate a port based on the app id

    Validate that the port is an integer between 1024 and 65535

    Start a Node server and try to bind to the port

    If we can bind, the port is available and we kill the server and continue with the build

    If we cannot bind, then connect to the port to see if it's a Titanium app:

    If it's a Titanium app, check if the app id matches the one app we're building

    If yes, then continue with the build... when we install the new version, it'll kill the old one and release the port

    If no, then error out

    If it's not a Titanium app...

    and the remote server expects data upon connect...

    and the server times out the connection because of no activity

    and server hangs and we timeout the connection because of no activity

    and the remote server immediately sends us data, but the first line of data is not our JSON encoded log header

    To test:

    Create 2 apps

    Set a <log-server-port>6666</log-server-port> in the tiapp.xml

    Build app1 for simulator

    Leave app running

    Build app2 for simulator

    App2 should fail to build:

       [ERROR] Another Titanium app "com.appcelerator.testapp" is currently running and using the log server port 6666
       [ERROR] Stop the running Titanium app, then rebuild this app
       [ERROR] -or-
       [ERROR] Set a unique <log-server-port> between 1024 and 65535 in the <ios> section of the tiapp.xml
       

    Kill App1 and it's build

    Start a node server on port 6666

    require('net').createServer(function () {
       	console.log('got a new connection!');
       }).listen({
       	host: 'localhost',
       	port: 6666
       }, function () {
       	console.info('Server started');
       });

    Build App1 or app2

       [ERROR] Another process is currently bound to port 6666
       [ERROR] Set a unique <log-server-port> between 1024 and 65535 in the <ios> section of the tiapp.xml
       
    For bonus points, install something like MongoDB (27017) or MySQL (3306), set the <log-server-port>, and build. Should be same as last error above. TiSDK master PR: https://github.com/appcelerator/titanium_mobile/pull/8515 TiSDK 6_0_X PR: https://github.com/appcelerator/titanium_mobile/pull/8516
  4. Chee Kiat Ng 2016-10-17

    [~cbarber] the test cases work fine but i don't think it fixed differential build. When I initiated the subsequent build, sure enough there's a lot of "No change, skipping ...", but it still says "Invoking xcodebuild". Last I remember, it should say "Skipping xcodebuild". Please let me know if this is expected behavior.
  5. Chris Barber 2016-10-17

    [~cng] It will say in the log output what is triggering xcodebuild, so what does it say?
  6. Chee Kiat Ng 2016-10-17

    [~cbarber] Strangely enough it is saying [INFO] :   Forcing rebuild: image was updated, recompiling asset catalog but i didn't do any image change. See attached 2 trace files.
  7. Chris Barber 2016-10-17

    Then that means that the Xcode project and thus the log server port are not the reason for the rebuilds in which case this ticket is fixed. There obviously is a new issue surrounding asset catalog images. You could trying disabling app thinning and see if the problem is exclusively related to asset catalog images and file a ticket if so.
  8. Chee Kiat Ng 2016-10-18

    PRs merged. CR and FT passed.
  9. Eric Merriman 2018-08-06

    Cleaning up older fixed issues. If this issue should not have been closed as fixed, please reopen.

JSON Source