Replace runghc with ghc -e Main.main

[snoyberg]

Motivation: if you call runghc, that process continues to live on
separate from the actual ghc process that interprets the code. That's
inefficient. But more worryingly: the runghc process itself can cause
major problems in Docker images, since it ends up swallowing SIGINTs as
the PID-1 process. This was discussed quite a bit on Twitter:

https://twitter.com/argumatronic/status/763418990982991872

[borsboom]

Copied from Slack:

This is a good change no matter what. That said, for the Docker case, maybe we should go back to having a separate 'init' process. We used to use phusion/baseimage's my_init in the containers but eliminated that for faster startup and compatibility with non-FP Complete-generated images. At that time we considered whether we should introduce our own extra 'init' process but decided to try without and see how it went. Up to this issue, it doesn't seem to have caused any problems but now here's a case where it did.

[snoyberg]

Also copied from Slack:

In the case that I ran into, I wasn't actually launching the Docker image with Stack, so it wouldn't be relevant.

[Blaisorblade]

Know this is semi-OT but:

We used to use phusion/baseimage's my_init in the containers but eliminated that for faster startup and compatibility with non-FP Complete-generated images.
In the case that I ran into, I wasn't actually launching the Docker image with Stack, so it wouldn't be relevant.

But Docker's initless containers will have the same issues with signal handling. Have you considered using https://github.com/krallin/tini? If there's interest I can split an issue for this.

[mgsloan]

Makes sense to me!

[snoyberg]

I ended up using dumb-init to solve the problem, which is the same concept
as tini.

On Fri, Aug 12, 2016, 3:10 AM Michael Sloan [email protected]
wrote:

Merged #2474 #2474.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2474 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADBB9JK0DCqO-2IgVGziVAPsg6WyMliks5qe7msgaJpZM4JiCSl
.

[harendra-kumar]

With this fix we lost the ability to pass arguments to scripts. One possible fix is to use:

ghc -- -e 'System.Environment.withArgs [<args>] Main.main'  <file>

Or to revert to using runghc now that the original problem has been solved differently.

[snoyberg]

That's a good catch, I didn't realize that's how ghc -e worked. I'd say revert my patch then.

[harendra-kumar]

Ok, I will revert back to using runghc then.

[snoyberg]

Thanks, sorry for the breakage. Going forward, we can consider either:

• Doing the more complicated approach you described, which may be
  worthwhile.
• Pushing a fix upstream to GHC itself to exec instead of fork/exec
  the new process. I can probably start that off with a fix in the process
  package.

On Tue, Aug 16, 2016 at 4:58 PM, Harendra Kumar [email protected]
wrote:

Ok, I will revert back to using runghc then.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2474 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADBB9-ye1HxfG8UH2tKqgHU2D3bmA7Zks5qgcIdgaJpZM4JiCSl
.

[harendra-kumar]

Well, it seems ghc -e is nothing but ghci evaluation and in fact even uses .ghci as well by default. So we can use ghc -e ':set args <args>' -e 'Main.main'.

But there are a bit more issues which runghc takes care of and if we plan to take care of those ourselves then it means we are just replicating runghc. Some of the things that it takes care of:

• interpret stdin
• ignore .ghci
• set progname correctly

Though its trivial to take care of all of them except interpreting stdin. Still it may be a good idea to use runghc itself so that we do not have to maintain the code. We can fix the original problem by using exec as you suggested in the second option above.

[Blaisorblade]

To be sure: you're talking just about inefficiency, not signal handling (for which we need a real init, see #2498), right? If so, I agree on approach n. 2.

[harendra-kumar]

Yeah I guess there is no other general way for proper signal handling other than using a tailor made process.

I think we need to keep the parent runghc process around to cleanup the temporary file that runghc creates to interpret stdin. Though it may be possible to use delete-on-close to clean up the temp file automatically even without keeping the parent around. In the non-stdin case there is no reason for the parent to be around. So I think in both cases we could do with one process instead of two.

I think this optimization is a nice to have and could be useful only when we are running out of process slots or using too many of them. Not sure if it has any use in this particular use case, definitely not for performance.