#7231 closed bug (fixed)

GHCi erroneously unloads modules after a failed :reload

Reported by: parcs Owned by: simonmar
Priority: high Milestone: 7.6.2
Component: GHCi Version: 7.6.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time performance bug Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description


Say you have three modules:

module Main where

import Foo
import Bar
module Foo where
module Bar where

And you run the following in GHCi:

:load Main
:! echo '"' >> Bar.hs # alter Bar in such a way that it fails to compile
:reload

What I expect to have happened is that Bar fails to compile and so does Main (because Main imports Bar), thus Bar and Main should get unloaded. So in the end Foo should be the only module left loaded.

What I've observed is that not only do Bar and Main get unloaded, but Foo does too! Yet Foo should not get unloaded because it has been altered.

Change History (8)

comment:1 Changed 20 months ago by parcs

Oops, the last sentenced is supposed to be "Yet Foo should not get unloaded because it has not been altered."

comment:2 Changed 20 months ago by parcs

I have appended a potential fix for this issue. Instead of loading modules in a topological order, modules are sorted according to their stability (stable modules are loaded before unstable ones) followed by their topological order. It should be safe to load all stable modules before all unstable modules because stable modules can only depend on other stable modules.

diff --git a/compiler/main/GhcMake.hs b/compiler/main/GhcMake.hs
index 322c631..dd3633c 100644
--- a/compiler/main/GhcMake.hs
+++ b/compiler/main/GhcMake.hs
@@ -238,11 +238,16 @@ load how_much = do
         stable_mg = 
             [ AcyclicSCC ms
             | AcyclicSCC ms <- full_mg,
-              ms_mod_name ms `elem` stable_obj++stable_bco,
-              ms_mod_name ms `notElem` [ ms_mod_name ms' | 
-                                            AcyclicSCC ms' <- partial_mg ] ]
+              ms_mod_name ms `elem` stable_obj++stable_bco ]
 
-        mg = stable_mg ++ partial_mg
+        unstable_mg =
+            [ AcyclicSCC ms
+            | AcyclicSCC ms <- partial_mg,
+              ms_mod_name ms `notElem` stable_obj++stable_bco ]
+
+        -- Load all the stable modules first, before attempting to load
+        -- an unstable module (#7231).
+        mg = stable_mg ++ unstable_mg
 
     -- clean up between compilations
     let cleanup hsc_env = intermediateCleanTempFiles dflags

comment:3 Changed 20 months ago by parcs

I should probably explain how I arrived at this fix: sometimes an unstable module appears before a stable module in the module graph. When it comes time to load the unstable module, and if loading it fails, then the stable module that comes after it doesn't get loaded, though it should have been.

So in the scenario outlined in the ticket description, the module graph looks like this:

mg = [ Bar (unstable), Foo (stable), Main (unstable) ]

and during the upsweep Bar will fail to load, so both Foo and Main get discarded. With the patch the module graph will look like:

mg = [ Foo (stable), Bar (unstable), Main (unstable) ]

and during the upsweep Foo will load successfully, and Bar will fail to load, so only Main gets discarded.

comment:4 Changed 19 months ago by simonmar

  • Difficulty set to Unknown
  • Milestone set to 7.6.2
  • Owner set to simonmar
  • Priority changed from normal to high

I'll look into this.

comment:5 Changed 17 months ago by marlowsd@…

commit 086d7c54f5bddbc9e5d94a9ae9c4b5aeeab53a35

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Tue Nov 27 08:55:31 2012 +0000

    Fix #7231: don't unload stable modules when there is an error later

 compiler/main/GhcMake.hs |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

comment:6 Changed 17 months ago by simonmar

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

comment:7 Changed 17 months ago by simonmar

  • Status changed from closed to merge

comment:8 Changed 17 months ago by igloo

  • Status changed from merge to closed
Note: See TracTickets for help on using tickets.