Bug 241849 - lang/ghc: Enable USE_PTHREAD_FOR_ITIMER
Summary: lang/ghc: Enable USE_PTHREAD_FOR_ITIMER
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-haskell (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-10 02:27 UTC by Kevin Zheng
Modified: 2020-05-26 20:58 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (haskell)

Attachments
Patch (719 bytes, patch)
2019-11-10 02:27 UTC, Kevin Zheng 		no flags Details | Diff
Patch (675 bytes, patch)
2020-05-25 18:31 UTC, Kevin Zheng 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Zheng 2019-11-10 02:27:52 UTC 
Created attachment 209025 [details]
Patch

The GHC RTS has an interval timer that is apparently used both in the threaded (-threaded) and non-threaded runtime.

On some operating systems, rts/posix/Itimer.c sets USE_PTHREAD_FOR_ITIMER, which uses usleep() at regular intervals instead of SIGVALRM.

Per the comments about Darwin (macOS) above:

 * We want to avoid using the SIGALRM signals whenever possible as these signals
 * interrupt system calls (see #10840) and can be overridden by user code. On
 * Darwin we can use a dedicated thread and usleep.

The attached patch applies this for FreeBSD as well.

To see the difference, run 'truss' on a simple test program:

== test.hs ==
main = do
    c <- getChar
    putChar c

$ ghc test.hs
$ truss ./test.hs
... some stuff
SIGNAL 26 (SIGVTALRM) code=SI_TIMER value=0x0 timerid=3 overrun=0
sigprocmask(SIG_SETMASK,{ SIGVTALRM },0x0)       = 0 (0x0)
sigreturn(0x7fffffff9880)                        = 0 (0x0)
ioctl(0,TIOCGETA,0x7fffffffa270)                 = 0 (0x0)
poll({ 0/POLLIN },1,0)                           = 0 (0x0)
select(1,{ 0 },{ },0x0,0x0)                      ERR#4 'Interrupted system call'
SIGNAL 26 (SIGVTALRM) code=SI_TIMER value=0x0 timerid=3 overrun=0
sigprocmask(SIG_SETMASK,{ SIGVTALRM },0x0)       = 0 (0x0)
sigreturn(0x7fffffffd6c0)                        ERR#4 'Interrupted system call'
select(1,{ 0 },{ },0x0,0x0)                      ERR#4 'Interrupted system call'
... more of this

After this change:
... some stuff
ioctl(0,TIOCGETA,0x7fffffffa270)                 = 0 (0x0)
poll({ 0/POLLIN },1,0)                           = 0 (0x0)
nanosleep({ 0.010000000 })                       = 0 (0x0)
nanosleep({ 0.010000000 })                       = 0 (0x0)
nanosleep({ 0.010000000 })                       = 0 (0x0)
... more of this
select(1,{ 0 },{ },0x0,0x0)                      = 1 (0x1)
... more stuff
Comment 1 Kevin Zheng 2019-11-10 02:28:53 UTC 
Perhaps a PORTREVISION bump is warranted. The RTS will need to be re-linked with any Haskell binaries.
Comment 2 Gleb Popov freebsd_committer freebsd_triage 2019-11-10 07:03:18 UTC 
Thanks. Did you upstream this?
Comment 3 Kevin Zheng 2019-11-10 09:23:58 UTC 
(In reply to Gleb Popov from comment #2)
Not yet. I don't track upstream GHC, and wanted to get feedback from haskell@ first.
Comment 4 Gleb Popov freebsd_committer freebsd_triage 2019-11-18 10:04:49 UTC 
Kevin, your patch has been submitted upstream. Do you want me to add this patch to GHC 8.6.5 or you're OK with waiting for GHC 8.8.2, which already have it?
Comment 5 Kevin Zheng 2019-11-18 16:14:30 UTC 
(In reply to Gleb Popov from comment #4)
Thank you!

It doesn't really matter to me, because I have the change applied locally. I understand if there's not really a desire to do a PORTREVISION bump.
Comment 6 Gleb Popov freebsd_committer freebsd_triage 2019-12-11 07:13:13 UTC 
The change will be present in upcoming GHC 8.8.2, so I'm closing this.

Thanks everyone involved.
Comment 7 Kevin Zheng 2020-05-25 18:31:35 UTC 
Created attachment 214847 [details]
Patch

Hi again,

This issue is still present in the ghc-8.8.3 in ports. Some quick investigation reveals that USE_PTHREAD_FOR_ITIMER is only set if THREADED_RTS is defined; that is, when ghc is run with the `-threaded` option:

https://gitlab.haskell.org/ghc/ghc/-/commit/ec8a463d1ff948ba9b1b0fbb538f7d5a237bf44a

From the context, Linux is the only OS where USE_PTHREAD_FOR_ITIMER depends on THREADED_RTS also being set. This seems to be because on Linux, GHC uses some timerfd thing.

FreeBSD, like the rest of the OS's, should always define USE_PTHREAD_FOR_ITIMER. The patch to correct this locally is attached. I will defer to haskell@, but would prefer this to be applied locally with a PORTREVISION bump because of the long time it takes to update a version from upstream.
Comment 8 Gleb Popov freebsd_committer freebsd_triage 2020-05-26 15:16:40 UTC 
Submitted upstream: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3360

I'll also include the patch in the ports tree.
Comment 9 Gleb Popov freebsd_committer freebsd_triage 2020-05-26 15:41:29 UTC 
Kevin, have you verified that the patch doesn't break anything for non-threaded case?
Comment 10 Kevin Zheng 2020-05-26 17:36:07 UTC 
(In reply to Gleb Popov from comment #9)
I don't think this breaks anything in the non-threaded case, but my testing has been limited to myself.

I've tested this on a few small example programs as well as cabal installing xmonad. An extra pair of eyes on potential breakage might be good before it really hits the ports tree. Because the pthread itimer is default on other platforms, I suspect the risk is low.
Comment 11 commit-hook freebsd_committer freebsd_triage 2020-05-26 20:19:37 UTC 
A commit references this bug:

Author: arrowd
Date: Tue May 26 20:18:42 UTC 2020
New revision: 536651
URL: https://svnweb.freebsd.org/changeset/ports/536651

Log:
  lang/ghc: Add patch for timer machinery.

  PR:		241849
  Submitted by:	Kevin Zheng <kevinz5000@gmail.com>

Changes:
  head/lang/ghc/Makefile
  head/lang/ghc/files/patch-rts_posix_Itimer.c
Comment 12 Gleb Popov freebsd_committer freebsd_triage 2020-05-26 20:20:12 UTC 
Thank you for working on this.
Comment 13 Kevin Zheng 2020-05-26 20:58:36 UTC 
(In reply to Gleb Popov from comment #12)
My pleasure; thanks also to you and haskell@!