Running Task causes crash

bwillis commented 10 years ago

I am not an expert in Obj-c at all, but tried to track down an exception that was being caused. This was the code I had in index.html to reproduce the issue:

<!DOCTYPE html>
<html>
<head>
  <title>Hello</title>
</head>
<body>

  <script type="text/javascript" charset="utf-8">
      var myTask = MacGap.Task.create('/usr/bin/say', function() {});
      myTask.arguments = ['hello there'];
      myTask.pipeOutput = true;
      myTask.launch();
  </script>
</body>
</html>

This would cause the application to crash with this exception:

2014-07-20 20:16:25.101 MG[23204:303] -[Task setOnError:]: unrecognized selector sent to instance 0x6000000bf800
2014-07-20 20:16:25.102 MG[23204:303] An uncaught exception was raised

It seems to be setting the onError attribute of the Task to nil that is causing the issue. I made an assumption here that it is not being or will be used, so my PR is to remove the onError attribute.

Please let me know if this seems correct, thanks!

rawcreative commented 10 years ago

The code you posted will always crash. The is because you are setting 'pipeOutput = true' for a process/cmd that does not return anything.

NSTask does not have any type of safe-handling for this, if you tell it to expect output from the task, it will, if there is no output, the thread will hang waiting for a response and the app will eventually crash, there unfortunately is no way around this. pipeOutput should only be used when the command you're running will return a value of some kind (even if it's false/0/-1/etc)

bwillis commented 10 years ago

@rawcreative thanks for the reply.

I have tested without the myTask.pipeOutput = true; as well as with this example I found in issues and they both crash the same way:

var myTask = MacGap.Task.create('/bin/ls', function() {});
myTask.pipeOutput = true;
myTask.launch();

This is latest master-maybe I have missed something obvious, but it consistently crashes for me. With the proposed patch, both examples complete successfully. Even the first say command with the pipeOutput set.

rawcreative commented 10 years ago

I'm not sure why that code is in master as I don't have it in my repo, maybe an unintended bi-product of some other merge that wasn't caught. Adding it does in fact cause a crash so I'll merge this to clean up master.

bwillis commented 10 years ago

Awesome thanks! :+1:

MacGapProject / MacGap2

Running Task causes crash #35