Skip to content

fix SMJobSubmit fail on some osx#197

Closed
sunuslee wants to merge 2 commits into
Squirrel:masterfrom
sunuslee:master
Closed

fix SMJobSubmit fail on some osx#197
sunuslee wants to merge 2 commits into
Squirrel:masterfrom
sunuslee:master

Conversation

@sunuslee

@sunuslee sunuslee commented Jan 3, 2017

Copy link
Copy Markdown

SMJobSubmit will fail on some osx(tested in 10.11).
when this happened, the job will be shown in the launchctl list
but will never be executed (ShipIt not running) and the process
will be terminated by later [NSApp terminate:self] as expected.
so we manually launchctl start the ShipIt job.

Signed-off-by: sunuslee sunuslee@gmail.com

SMJobSubmit will fail on some osx(tested in 10.11).
when this happened, the job will be shown in the launchctl list
but will never be executed (ShipIt not running) and the process
will be terminated by later [NSApp terminate:self] as expected.
so we manually launchctl start the ShipIt job.

Signed-off-by: sunuslee <sunuslee@gmail.com>
Signed-off-by: sunuslee <sunuslee@gmail.com>
@joshaber joshaber self-assigned this Jan 9, 2017
@joshaber

joshaber commented Jan 9, 2017

Copy link
Copy Markdown
Contributor

I'm pretty conservative about making changes to Squirrel.Mac, given:

  1. The original maintainers are gone.
  2. A lot of projects depend on it.

So to consider this PR, I'll need a more thorough description of the bug and how this is addressing it.

}

// wait for ShipIt to start.
sleep(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to open us up to a race condition.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

during our own test, there might be some time before launchctl submit bring up ShipIt binary. so we just wait , for now.
otherwise, the check for shipIt is running might get the wrong result. but the update will still work. because what we did in the code is run another shipit.


+ (BOOL)isShipItRunning {
NSTask *checkShipItRunningTask = [[NSTask alloc] init];
checkShipItRunningTask.launchPath=@"/usr/bin/pgrep";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use NSRunningApplication or NSWorkspace or something higher-level than shelling out to pgrep?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, i am aware of NSRunningApplication and NSWorkspace. but ShipIt is a command line tool. it can not be found by [[NSWorkspace sharedWorkspace]runningApplications]; and it does not have a valid bundle id. so, we have to search by it's name.
correct me if i am wrong:)

checkShipItRunningTask.launchPath=@"/usr/bin/pgrep";
checkShipItRunningTask.arguments = @[@"ShipIt"];
[checkShipItRunningTask launch];
[checkShipItRunningTask waitUntilExit];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be blocking.

sleep(1);

if (![self isShipItRunning]) {
// ShipIt not running, possible something wrong with SumJobSubmit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: SMJobSubmit

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx man

@sunuslee

Copy link
Copy Markdown
Author

@joshaber when this bug happened , if the user finished downloading the new update file, the app will quit and won't launch again. because it's ShipIt's responsibility to bring up the app, but ShipIt can not start running in the first place, because SMJobSumit failed.

@joshaber

Copy link
Copy Markdown
Contributor

but ShipIt can not start running in the first place, because SMJobSumit failed

But why does it fail?

@sunuslee

Copy link
Copy Markdown
Author

@joshaber
i wish i knew the answer. but it seems like an apple's bug to me.
when you run
launchctl submit -l test -p /usr/bin/touch -- /usr/bin/touch /var/tmp/test
it should be a file in /var/tmp/test and it will show you this job

$ launchctl list test

{
	"LimitLoadToSessionType" = "Aqua";
	"Label" = "test";
	"TimeOut" = 30;
	"OnDemand" = false;
	"LastExitStatus" = 0;
	"Program" = "/usr/bin/touch";
	"ProgramArguments" = (
		"/usr/bin/touch";
		"/var/tmp/test";
	);
};

but when this bug happened, this simple command will only show you the job in launchctl list , and not executing at all.
i wish i knew what's going on inside the system:(

@sunuslee

Copy link
Copy Markdown
Author

@joshaber can you offer a better solution?

@joshaber

Copy link
Copy Markdown
Contributor

I can't since I don't fully understand the bug or how it happens. But I'm hesitant to fix an apparent race condition by introducing another potential race condition.

@joshaber

Copy link
Copy Markdown
Contributor

Closing due to inactivity.

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.

2 participants