Opened 7 months ago

Last modified 5 months ago

#8303 new bug

defer StackOverflow exceptions (rather than dropping them) when exceptions are masked

Reported by: rwbarton Owned by: thoughtpolice
Priority: high Milestone: 7.8.3
Component: Runtime System Version: 7.7
Keywords: Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Easy (less than 1 hour)
Test Case: Blocked By:
Blocking: Related Tickets:

Description

See http://www.reddit.com/r/haskell/comments/1luan1/strange_io_sequence_behaviour/ for a very simple program (main') that accidentally evades the stack size limit, running to completion even though it has allocated hundreds of megabytes of stack chunks, and my comment for an explanation of this behavior.

ryani suggested that when a thread exceeds its stack limit but it is currently blocking exceptions, the RTS shouldn't simply drop the StackOverflow exception, but rather deliver it when the mask operation completes. That sounds sensible to me and it would give a nice guarantee that when any individual mask operation uses a small amount of stack, the stack size limit is approximately enforced.

(I know that the default stack size limit may go away or essentially go away, but it can still be nice when developing to use a small stack size limit, so that one's system isn't run into the ground by infinite recursion quickly gobbling up tons of memory.)

Attachments (2)

0001-If-exceptions-are-blocked-add-stack-overflow-to-bloc.patch (4.5 KB) - added by ezyang 7 months ago.
Validated patch
0001-Test-for-8303.patch (1.6 KB) - added by rwbarton 6 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 months ago by ezyang

  • Cc simonmar added
  • Difficulty changed from Unknown to Easy (less than 1 hour)
  • Owner set to ezyang
  • Version changed from 7.6.3 to 7.7

Posted an untested patch I am validating. It is a little bit of a hack, but not very much.

Changed 7 months ago by ezyang

Validated patch

comment:2 Changed 7 months ago by ezyang

  • Status changed from new to patch

Posted a new patch that validates.

comment:3 Changed 7 months ago by simonmar

This is a very clever trick, and it has a certain devious beauty. I have no idea if we're going to trip up an assumption somewhere by having a throwTo message where the source and the destination are the same thread, but I'm happy to take the risk (and I'm sure we can fix up any breakage that results).

comment:4 Changed 7 months ago by simonpj

"a clever trick, with devious beauty". I love the sound of that, but Edward can you make sure there is a clear Note [Clever trick of devious beauty] to document the intent and explain how it works? Thanks.

Simon

comment:5 Changed 6 months ago by rwbarton

Here is a test, adapted from the original program. I tested that it fails currently and passes if I replace (hPutStrLn h "Hi") by (return ()); I haven't tested your patch, Edward.

Changed 6 months ago by rwbarton

comment:6 Changed 6 months ago by ezyang

=====> T8303(normal) 59 of 3801 [0, 0, 0] 
cd ./rts && '/home/hs01/ezyang/ghc-stackoverflow/inplace/bin/ghc-stage2' -fforce-recomp -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-db -rtsopts -fno-ghci-history -o T8303 T8303.hs    >T8303.comp.stderr 2>&1
cd ./rts && ./T8303 +RTS -K2K -RTS   </dev/null >T8303.run.stdout 2>T8303.run.stderr

OVERALL SUMMARY for test run started at Fri Oct 11 11:24:21 2013 PDT
 0:00:02 spent to go through
    3801 total tests, which gave rise to
   14943 test cases, of which
   14942 were skipped

       0 had missing libraries
       1 expected passes
       0 expected failures

I'll go ahead and add extra docs, and then push.

comment:7 Changed 6 months ago by Edward Z. Yang <ezyang@…>

In c6af06ef38813c460eaa0dec613eae98f4a57796/ghc:

If exceptions are blocked, add stack overflow to blocked exceptions list. Fixes #8303.

Signed-off-by: Edward Z. Yang <ezyang@mit.edu>

comment:8 Changed 6 months ago by ezyang

Hey Reid, I wanted to use your test case, but its use of /dev/null makes it not portable. Can you fix that?

comment:9 Changed 6 months ago by rwbarton

Hey Reid, I wanted to use your test case, but its use of /dev/null makes it not portable.

Does it really? I figured it would be okay since I noticed the testsuite driver redirected stdin from /dev/null anyways... does it do something different on Windows?

Can you fix that?

Hmm, maybe. I guess since I set the max stack size so low, I can reduce bignum to something like 500, set ignore_output, and have the failure mode of the test be that the program completes successfully... I'll try it.

comment:10 Changed 6 months ago by ezyang

I believe the situation is that msys will provide /dev/null, but MingW compiled things will not. I can pull it up on my Windows box and test it out if you like.

comment:11 Changed 5 months ago by simonmar

  • Milestone set to 7.8.1
  • Owner ezyang deleted
  • Priority changed from normal to high
  • Status changed from patch to new

comment:12 Changed 5 months ago by simonmar

  • Owner set to thoughtpolice
Note: See TracTickets for help on using tickets.