Opened 3 years ago

Last modified 13 months ago

#8303 new bug

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

Reported by: rwbarton Owned by: thoughtpolice
Priority: low Milestone:
Component: Test Suite Version: 7.7
Keywords: Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

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 3 years ago.
Validated patch
0001-Test-for-8303.patch (1.6 KB) - added by rwbarton 3 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 3 years ago by ezyang

Cc: simonmar added
difficulty: UnknownEasy (less than 1 hour)
Owner: set to ezyang
Version: 7.6.37.7

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

Changed 3 years ago by ezyang

Validated patch

comment:2 Changed 3 years ago by ezyang

Status: newpatch

Posted a new patch that validates.

comment:3 Changed 3 years 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 3 years 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 3 years 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 3 years ago by rwbarton

Attachment: 0001-Test-for-8303.patch added

comment:6 Changed 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years ago by simonmar

Milestone: 7.8.1
Owner: ezyang deleted
Priority: normalhigh
Status: patchnew

comment:12 Changed 3 years ago by simonmar

Owner: set to thoughtpolice

comment:13 Changed 3 years ago by thoughtpolice

Priority: highlow

comment:14 Changed 3 years ago by thoughtpolice

Priority: lowhigh

comment:15 Changed 3 years ago by thoughtpolice

Priority: highlow

This is merged, the testsuite needs investigation, but I believe this should be fine with MSYS.

comment:16 Changed 3 years ago by thoughtpolice

Milestone: 7.8.37.8.4

Moving to 7.8.4.

comment:17 Changed 2 years ago by thoughtpolice

Milestone: 7.8.47.10.1

Moving (in bulk) to 7.10.4

comment:18 Changed 2 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:19 Changed 18 months ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:20 Changed 18 months ago by thomie

Component: Runtime SystemTest Suite

Bug is fixed. Just needs a test.

comment:21 Changed 13 months ago by thomie

Milestone: 8.0.1
Note: See TracTickets for help on using tickets.