Opened 2 years ago

Closed 3 months ago

#8435 closed bug (fixed)

Do not copy stack after stack overflow

Reported by: ezyang Owned by: archblob
Priority: normal Milestone: 7.10.2
Component: Runtime System Version: 7.7
Keywords: Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions: Phab:D938

Description

I am not sure, but I was doing a close reading of threadStackOverflow and noticed that after we throw a stack overflow exception, we fall through to the code responsible for allocating a new stack and copying the code over. An old iteration of the stack overflow code (removed in the commit cited below) did return after throwing the exception, and I did not see anything in the commit message suggesting the change was intentional. It seems sound to copy the stack; it will just become dead immediately.

The relevant commit:

commit f30d527344db528618f64a25250a3be557d9f287
Author: Simon Marlow <[email protected]>
Date:   Wed Dec 15 12:08:43 2010 +0000

    Implement stack chunks and separate TSO/STACK objects

Suggested (untested) patch:

diff --git a/rts/Threads.c b/rts/Threads.c
index 742119d..ccd6b17 100644
--- a/rts/Threads.c
+++ b/rts/Threads.c
@@ -533,6 +533,7 @@ threadStackOverflow (Capability *cap, StgTSO *tso)
 
         // Send this thread the StackOverflow exception
         throwToSingleThreaded(cap, tso, (StgClosure *)stackOverflow_closure);
+        return;
     }
 
 

Change History (10)

comment:1 Changed 23 months ago by simonmar

Sounds reasonable to me. If it validates, go ahead and push.

comment:2 Changed 23 months ago by Edward Z. Yang <ezyang@…>

In 9fb30cbcbc02086c5c6eb3942acdfdcad8331cb9/ghc:

Do not copy stack after stack overflow, fixes #8435

Signed-off-by: Edward Z. Yang <[email protected]>

comment:3 Changed 23 months ago by ezyang

  • Resolution set to fixed
  • Status changed from new to closed

comment:4 Changed 3 months ago by archblob

  • Milestone set to 7.10.2
  • Owner simonmar deleted
  • Resolution fixed deleted
  • Status changed from closed to new

This change was reverted in d70b19bfb5ed79b22c2ac31e22f46782fc47a117 and is the reason for the difference between 7.10.1 and 7.8.4 outpus in #10445.

comment:5 Changed 3 months ago by archblob

  • Differential Revisions set to Phab:D938

comment:6 Changed 3 months ago by archblob

  • Status changed from new to patch

comment:7 Changed 3 months ago by archblob

  • Owner set to archblob

comment:8 Changed 3 months ago by Austin Seipp <austin@…>

In 892c3e98bcef50aa56ec818f4d001aee36e05bbc/ghc:

Do not copy stack after stack overflow, refix #8435

Summary:
This was reverted in d70b19bfb5ed79b22c2ac31e22f46782fc47a117
and is a part of the reason for #10445.

Test Plan: validate

Reviewers: ezyang, simonmar, austin

Reviewed By: simonmar, austin

Subscribers: bgamari, thomie

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

GHC Trac Issues: #8435

comment:9 Changed 3 months ago by thoughtpolice

  • Status changed from patch to merge

comment:10 Changed 3 months ago by thoughtpolice

  • Resolution set to fixed
  • Status changed from merge to closed

Merged to ghc-7.10.

Note: See TracTickets for help on using tickets.