Opened 17 months ago

Closed 5 weeks ago

#13167 closed bug (fixed)

GC and weak reference finalizers and exceptions

Reported by: Yuras Owned by:
Priority: normal Milestone: 8.6.1
Component: Compiler Version: 8.0.1
Keywords: Cc: simonmar, Phyx-
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4693
Wiki Page:


When GC runs a number of finalizers in a row, and the first of them throws an exception, then other finalizers are ignored. The relevant piece of code is here.

The following program reproduces the issue:

import Data.IORef
import Control.Monad
import Control.Exception
import System.Mem

main :: IO ()
main = do

run :: IO ()
run = do
  ref <- newIORef ()
  void $ mkWeakIORef ref $ do
    putStr "."
    throwIO $ ErrorCall "failed"

I expect it to output "....", but I get only "."

The issue makes it unsafe to rely on finalizer for resource cleanup because unrelated finalizer (e.g. from some other library) may prevent your finalizer from running.

Actually I was sure the issue is known, but today I tried to find a reference to it, and failed.

If it is by design, then it should be documented somewhere.

Change History (9)

comment:1 Changed 17 months ago by simonpj

Cc: simonmar added

Adding simonmar who is king of finalisers.

comment:2 Changed 17 months ago by simonmar

Yes, this is technically a bug. We could have runFinalizerBatch catch and discard exceptions (or print them to stdout; it's really a bug if a finalizer throws an exception).

comment:3 Changed 11 months ago by andrewthad

I've open a PR that adds docs explaining this behavior.

comment:4 in reply to:  2 Changed 6 weeks ago by Phyx-

Cc: Phyx- added

Replying to simonmar:

Yes, this is technically a bug. We could have runFinalizerBatch catch and discard exceptions (or print them to stdout; it's really a bug if a finalizer throws an exception).

Any suggestions on how? the I/O manager seems to rely on using finalizers to flush buffers, however for some reason my new I/O manager's finalizers never get called. I assume because something else threw an exception.

I've modified runFinalizerBatch to handle exceptions (unless I misunderstood the code)

runFinalizerBatch :: Int -> Array# (State# RealWorld -> State# RealWorld)
                  -> IO ()
runFinalizerBatch (I# n) arr =
   let  go m  = IO $ \s ->
                  case m of
                  0# -> (# s, () #)
                  _  -> let !m' = m -# 1# in
                        case indexArray# arr m' of { (# io #) ->
                        case catch# (\p -> (# io p, () #)) (\_ s'' -> (# s'', () #)) s of          {
                         (# s', _ #) -> unIO (go m') s'
        go n

With mio the output varies between .. and ...., so not very deterministic. with winio none of it ever runs...

Any ideas what else might be making these handlers not run at all? or how I can debug this? for winio I can imagine one of the finalizers deadlocking, an ffi call maybe. But the mio output is strange.

If we're going to rely on them for managing handles and buffers they would ideally be a bit more robust...

comment:5 Changed 6 weeks ago by simonmar

The example program in the description doesn't wait for the finalizer threads to complete, so it might exit while there are still finalizers alive. This is why you don't see deterministic output. Probably adding a threadDelay 1000000 at the end would be enough.

A deadlock sounds like a plausible explanation for the behaviour you're seeing.

The fix to runFinalizerBatch looks good - we should probably do that anyway (I'm sure you could construct a test case to demonstrate the problem that it fixes).

comment:6 Changed 6 weeks ago by Phyx-

ah indeed, threadDelay worked!

Ok I'll whip up a testcase and patch.

I'll see if I can't add some debug code to figure out which finalizer is deadlocking.


comment:7 Changed 5 weeks ago by Phyx-

Differential Rev(s): Phab:D4693
Status: newpatch

comment:8 Changed 5 weeks ago by Ben Gamari <ben@…>

In 01bb17fd/ghc:

Make finalizers more reliable.

Ignore any errors thrown by finalizers when running them.

This prevents a faulty finalizer from stopping the rest being called.

Test Plan: ./validate, new test T13167

Reviewers: hvr, bgamari, simonmar

Reviewed By: bgamari, simonmar

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #13167

Differential Revision:

comment:9 Changed 5 weeks ago by Phyx-

Milestone: 8.6.1
Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.