"awaitSignal Nothing" does not block thread with -threaded

[adept]

Consider the following code:

module Main where

import System.Posix.Signals
import Control.Concurrent

main :: IO ()
main = do
  awaitSignal Nothing >> yield

According to the System.Posix.Signals haddock, "awaitSignal Nothing" should call pause(2), causing main thread to block until some signal has arrived. This is indeed what happens when this code is compiled by GHC 7.0.1 like so: "ghc --make awaitSignal.hs".

However, when compiled with -threaded, this code exits immediately when run. I think this is a bug in -threaded runtime.

I've tested with GHC 6.12.1, and this behavior is present there as well. Maybe I'm missing something non-obvious here?

My environment: x86, Debian sid, GHC 6.12.1 from Debian packages, GHC 7.0.1 from binary packages from GHC site.

[adept]

Debian sid, GHC 6.12.3 from the binary package: same behavior (exits with "-threaded").

Versions of unix package:

6.12.1: 2.4.0.0
6.12.3: 2.4.0.2
7.0.1:  2.4.1.0

[igloo]

Thanks for the report.

Looks like it's seeing internal SIGVTALRMs.

[adept]

Indeed it does. Running with +RTS -V0 gets the "proper" behavior.

[adept]

Attached patch should fix this. Please review.

[adept]

Masking scheduler timer signal under threaded RTS

[simonmar]

These CPP conditionals won't work in the .hsc file, I think, because the symbols aren't defined there. Also, the package is not build multiple times for threaded/unthreaded, so it's not meaningful to check for THREADED_RTS.

+#if defined(USE_TIMER_CREATE)
+      virtualTimerExpired
+#elif defined(HAVE_SETITIMER)
+#  if defined(THREADED_RTS) || !defined(HAVE_SETITIMER_VIRTUAL)
+      realTimeAlarm
+#  else
+      virtualTimerExpired
+#  endif
+#endif

[adept]

Well, apart from THREADED_RTS everything else will be available via Rts.h -> MachDeps?.h -> ghcautoconf.h, is it not?

Contents of the dist-install/build/System/Posix/Signals.hs support my theory: note that in the absence of needed defines, the whole #if .. #endif block would evaluate to empty. However, on my notebook, it evaluates to the (proper) value of virtualTimerExpired.

However, I agree that at the very least, check for defined(THREADED_RTS) should be removed. Modified patch will be attached after this comment.

[adept]

Excluded check for THREADED_RTS, import rtsSupportsBoundThreads from Control.Concurrent

[simonmar]

Are we really sure we should fix it this way? Perhaps instead we should document that the RTS uses SIGVTALRM or SIGALRM, and the caller should include those in the signal set unless they really know what they're doing.

[simonmar]

Oh, a better alternative might be this:

-- | A set of signals reserved for use by the implementation.  In GHC, this will normally
-- include either `sigVTALRM` or `sigALRM`.
reservedSignals :: SignalSet

And the documentation for awaitSignals would suggest using awaitSignals (Just reservedSignals)

This could be implemented using an RTS API like

int getTimerSignal(void);

to avoid all the #ifdef stuff, and keep the choice about which signal is reserved in just one place.

[adept]

Please note that awaitSignals (Just reservedSignals) would use just reservedSignals as signal mask, instead of adding them to the current process mask.

Let's look at the problem from this perspective: suppose that we set up a bunch of signal handlers and now we want to block until the signal arrives. Right now documentation says to use awaitSignal Nothing.

Unfortunately, awaitSignal Nothing is completely useless with "-threaded".

So, in order to write robust code that would work either way we would have to do something like this (and add this example to the documentation of "awaitSignal"):

{-# LANGUAGE CPP #-}
import Control.Concurrent (rtsSupportsBoundThreads)
import System.Posix.Signals

blockTillSignal = do
  sigset <- getSignalMask
  let sigset' =
        case rtsSupportsBoundThreads of
          False -> sigset
          True -> unionSignalSets sigset reservedSignals -- i'm making this up, no function like this right now.
  awaitSignal sigset'

Now, as you can see, this has large overlap with awaitSignal code. I think that this is a job for the library to hide away exactly this sort of complexity.

I might have a case of tunnel vision, but it seems like awaitSignal Nothing has a very limited use right now, unless one writes the code and is 100% sure that it would be used without "-threaded", which is simply not right.

[adept]

Or, let me put it this way: awaitSignal should provide a convenient interface to sigsuspend, right?

AFAIK, in C land the most common use cases for sigsuspend is to set up some signal handlers and then either:

1. Block until any of the signals arrive (using empty signal mask)

2. Block until one of the subset of the handled signals arrive (using non-empty signal mask)

For the latter, we have awaitSignal (Just someMask). For former, we have nothing. Rather we have a "honey trap" - if the programmer uses awaitSignal Nothing, his code will behave strangely with "-threaded" without any indication of what went wrong.

Hence I propose to restore the utility of awaitSignal Nothing via masking the scheduler signal away, more or less like the proposed patch does.

On the other hand, we should export reservedSignals like you proposed so that programmers may check when setting signal handlers if they are likely to step on RTS toes.

How does this sound?

[adept]

Ok. Following the discussion on IRC I've clarified the behavior of awaitSignal Nothing and exported the value of the timer signal from RTS. I hope that I did this properly - this is my "first export", so please check carefully :)

I verified and

blockSignals reservedSignals
awaitSignal Nothing

behaves as expected and as advertised with -threaded and without.

[simonmar]

Applied, thanks:

ghc patch:

Wed Dec  8 10:37:55 PST 2010  Dmitry Astapov <dastapov@gmail.com>
  * Export the value of the signal used by scheduler (#4504)

libraries/unix patch:

Wed Dec  8 18:38:49 GMT 2010  Dmitry Astapov <dastapov@gmail.com>
  * Clarify behavior of "awaitSignal Nothing", export SignalSet that includes all signals reserved by RTS (#4504)

[simonmar]

one more patch:

Fri Dec 10 01:00:45 PST 2010  Simon Marlow <marlowsd@gmail.com>
  * Fix Windows build: move rtsTimerSignal to the POSIX-only section

[kazu-yamamoto]

Why are these patch applied only to GHC HEAD? Why don't you apply these to ghc-7.0 branch?

[simonmar]

Replying to kazu-yamamoto:

Why are these patch applied only to GHC HEAD? Why don't you apply these to ghc-7.0 branch?

It's an API addition, and we don't normally change APIs on the stable branch. If this is important to you we could make an exception, though.

[kazu-yamamoto]

If this patch does not break anything, please include to 7.0.2. According to adept, this patch is necessary to run the "c10k" package well.

[simonmar]

Ian: let's merge this into 7.0.2.

[igloo]

All 3 merged

[kazu-yamamoto]

This bug appears again in GHC 7.0.3. I tested the code above both on Linux and Mac. awaitSignal does not block in both cases with -threaded.

[kazu-yamamoto]

It seemds to me that two patches out of 3 are not merged into GHC 7.0.3 nor GHC 7.0.4rc1. Is it possible to merge them to 7.0.4rc2?

[igloo]

All the patches looks merged to me.

If I understand correctly, the program needs to be changed to

module Main where

import System.Posix.Signals
import Control.Concurrent

main :: IO ()
main = do
  blockSignals reservedSignals
  awaitSignal Nothing >> yield

and that behaves correctly.

[kazu-yamamoto]

Ian,

Thank you for your reply and sorry for my misunderstanding. I comfirmed that adding "blockSignals reservedSignals" fixes this.