Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#10017 closed bug (fixed)

signal handlers are invoked multiple times when the threaded rts is used

Reported by: redneb Owned by: AndreasVoellmy
Priority: highest Milestone: 7.10.1
Component: Runtime System Version: 7.10.1-rc1
Keywords: Cc: AndreasVoellmy, simonmar, thoughtpolice
Operating System: Linux Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: #9423 Differential Rev(s): Phab:D641
Wiki Page:

Description

When you install a custom signal handler and the threaded rts is being used, then the signal handler will be invoked multiple times. Here's a program that the demonstrates this:

import Control.Concurrent
import System.Posix.Signals

main :: IO ()
main = do
    _ <- flip (installHandler sig) Nothing $ Catch $
        putStrLn $ "Received signal " ++ show sig
    raiseSignal sig
    threadDelay 100000
  where
    sig = sigUSR2

If you compile this with the -threaded flag and then run it with say +RTS -N4 then it produces the following output:

Received signal 12
Received signal 12
Received signal 12
Received signal 12
Received signal 12

In general the signal handler is invoked n_capabilities+1 times. This also happens with all other signals.

This regression was introduced by f9f89b7884ccc8ee5047cf4fffdf2b36df6832df (which was later reverted but then re-added), a commit addressing #9423.

The cause of the problem is this loop. I don't understand why we need to write an event about the signal received in the per capability control pipe introduced by the aforementioned commit. Aren't these control pipes supposed only to be used to shutdown the capabilities (which happens here)?

Removing the loop seems to solve the issue, but I don't know if it makes #9423 reappear. I cannot test this on a Mac OS X right now.

Change History (9)

comment:1 Changed 5 years ago by simonmar

Cc: thoughtpolice added
Milestone: 7.10.1
Priority: normalhighest

Yikes. Austin: this looks like a release blocker.

comment:2 Changed 5 years ago by AndreasVoellmy

I'm still getting up to speed, but it looks like that offending loop in generic_handler should just be removed. We have to signal to all the control pipes in ioManagerDie() (to solve the issue that led to that commit), but not in generic_handler (it seems). I'll look into it a bit more to check my understand and run some tests.

comment:3 Changed 5 years ago by AndreasVoellmy

Owner: changed from simonmar to AndreasVoellmy

comment:4 Changed 5 years ago by AndreasVoellmy

Differential Rev(s): Phab:D641

comment:5 Changed 5 years ago by AndreasVoellmy

In the threaded RTS, a signal is delivered from the RTS to Haskell user code by writing to file that one of the IO managers watches (via an instance of GHC.Event.Control.Control). When the IO manager receives the signal, it calls GHC.Conc.Signal.runHandlers to invoke Haskell signal handler. In the move from a single IO manager to one IO manager per capability, the behavior was (wrongly) extended so that a signal is delivered to every event manager (see Trac #9423), each of which invoke Haskell signal handlers, leading to multiple invocations of Haskell signal handlers for a single signal. This change fixes this problem by having the RTS (in generic_handler()) notify only the Control instance used by the TimerManager, rather than all the per-capability IO managers.

This has no impact on #9423; i.e. the fix here does not re-introduce the issue identified in #9423.

comment:6 Changed 5 years ago by Andreas Voellmy <andreas.voellmy@…>

In 92c93544939199f6ef758e1658149a971d4437c9/ghc:

Fix #10017

Summary:
In the threaded RTS, a signal is delivered from the RTS to Haskell
user code by writing to file that one of the IO managers watches (via
an instance of GHC.Event.Control.Control). When the IO manager
receives the signal, it calls GHC.Conc.Signal.runHandlers to invoke
Haskell signal handler. In the move from a single IO manager to one IO
manager per capability, the behavior was (wrongly) extended so that a
signal is delivered to every event manager (see #9423), each of which
invoke Haskell signal handlers, leading to multiple invocations of
Haskell signal handlers for a single signal. This change fixes this
problem by having the RTS (in generic_handler()) notify only the
Control instance used by the TimerManager, rather than all the
per-capability IO managers.

Reviewers: austin, hvr, simonmar, Mikolaj

Reviewed By: simonmar, Mikolaj

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D641

comment:7 Changed 5 years ago by AndreasVoellmy

Status: newmerge

comment:8 Changed 5 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Boom, headshot. Merged to ghc-7.10 via ddd95c0b575da33447c078a8791d3c4f2edec9b7.

comment:9 Changed 4 years ago by Simon Marlow <marlowsd@…>

In bb0e462b6cff02737d67f496d8172207042c22b5/ghc:

Mask to avoid uncaught ^C exceptions

Summary: It was possible to kill GHCi with a carefully-timed ^C

Test Plan: The bug in #10017 exposed this

Reviewers: bgamari, austin

Reviewed By: austin

Subscribers: thomie, bgamari

Differential Revision: https://phabricator.haskell.org/D1015

GHC Trac Issues: #10017
Note: See TracTickets for help on using tickets.