Trac field | Value |
---|---|

CC | - → carlostome |

After some investigation I found that indeed there is a deadlock between the mutexes of a TSO and a TVar. An excerpt of the debugging log is the follwing:

```
Task 2: cap 1: running thread 7012 (ThreadRunGHC)
Task 2: STM: 0xad6d08 : stmStartTransaction with 316 tokens
Task 2: STM: 0xad6d08 : stmStartTransaction()=0x4200224000
Task 2: STM: 0x4200224000 : stmWriteTVar(0x4200225c68, 0xad5e72)
Task 2: STM: 0x4200224000 : get_entry_for TVar 0x4200225c68
Task 2: STM: 0x4200224000 : FOR_EACH_ENTRY, current_chunk=0x420027e4d0 limit=0
Task 2: STM: 0x4200224000 : read_current_value(0x4200225c68)=0xad5e69
Task 2: STM: 0x4200224000 : stmWriteTVar done
Task 2: STM: 0x4200224000 : stmCommitTransaction()
Task 2: STM: 0x4200224000 : lock_stm()
Task 2: STM: 0x4200224000 : FOR_EACH_ENTRY, current_chunk=0x420027e4d0 limit=1
Task 2: STM: 0x4200224000 : trying to acquire 0x4200225c68
Task 2: STM: 0x4200224000 : cond_lock_tvar(0x4200225c68, 0xad5e69) -- (1)
Task 2: STM: 0x4200224000 : success
Task 2: STM: 0x4200224000 : doing read check
Task 2: STM: 0x4200224000 : FOR_EACH_ENTRY, current_chunk=0x420027e4d0 limit=1
Task 2: STM: 0x4200224000 : read-check succeeded
Task 2: STM: 0x4200224000 : FOR_EACH_ENTRY, current_chunk=0x420027e4d0 limit=1
Task 2: STM: 0x4200224000 : writing 0xad5e72 to 0x4200225c68, waking waiters -- (2)
Task 2: STM: unpark_waiters_on tvar=0x4200225c68
Task 2: STM: unpark_waiters_on tvar=0x4200225c68, status=0x4200223b80
Task 2: STM: cap 1: unpark_waiters_on tvar=0x4200225c68, status2=0x4200223b80 -- (3)
Task 2: STM: cap 1: unpark_tso id 6866 -- (4)
Task 1: BlockedOnSTM: lockTSO id 6866
Task 1: cap 0 BlockedOnSTM: raiseAsync id 6866
Task 1: cap 0: raising exception in thread 6866.
Task 1: cap 0: raiseAsync: freezing atomically frame -- (5)
Task 1: STM: 0x4200225f58 : stmAbortTransaction -- (6)
Task 1: STM: 0x4200225f58 : lock_stm()
Task 1: STM: 0x4200225f58 : aborting top-level transaction cap 0
Task 1: STM: 0x4200225f58 : aborting top-level transaction
Task 1: STM: 0x4200225f58 : stmAbortTransaction aborting waiting transaction
Task 1: STM: 0x4200225f58 : remove_watch_queue_entries_for_trec() -- (7)
Task 1: STM: 0x4200225f58 : FOR_EACH_ENTRY, current_chunk=0x4200223cc8 limit=1
Task 1: STM: 0x4200225f58 : lock_tvar (0x4200225c68) -- (8)
```

What is happening here has to do with how async exceptions are implemented.

The thread 6866 is waiting on the TVar 0x4200225c68. In the meantime the thread 7012 is able to change the value of such TVar to True. To do so, first it has to acquire the lock for the TVar (1), then actually change its value (2), and finally wake any thread that was waiting on this TVar (3). To wake up a concrete waiter it needs to hold its mutex through lock_tso() (4).

The problem is that when an async exception is thrown to a thread whose status is BlockedOnSTM, the first thing the thread throwing the exception does is to grab the mutex of the target thread with lock_tso() (https://github.com/ghc/ghc/blob/93220d46fceabf3afeae36f1fda94e1698c3639a/rts/RaiseAsync.c\#L419), then because the target thread was in the middle of an STM transaction (5) that dependes on the same TVar, it has to abort it (6) for what it has to be removed from the TVar watch queue (7) needing the mutex on the TVar to do so (8).

Basically the thread completing the STM grabs first the TVar lock and then the TSO lock while for the async exception the locks are taken in reverse order.

I'm still stuck on this, and right now I don't have that much time to work on it. I'll drop it for someone else to work on it.

The following program generates an invalid .hp file when compiled with ghc 8.2.1 but it does not when using ghc 8.0.2.

```
{-# LANGUAGE ScopedTypeVariables #-}
module Main where
eval :: forall a b. (a -> b -> b) -> b -> [a] -> b
eval f b xs = load xs []
where
load :: [a] -> [a] -> b
load [] stk = unload b stk
load (x:xs) stk = load xs (x : stk)
unload :: b -> [a] -> b
unload v [] = v
unload v (x : stk) = unload ((f $! x) $! v) stk
main :: IO ()
main = print (eval (||) False (True : replicate 10000000 False))
```

If strict application ($!) is substituted for normal application ($) or removed then the .hp generated file is correct.

For reproducing the error:

```
ghc -O2 --make -prof -fprof-auto Example.hs -fforce-recomp
./Example +RTS -hc
hp2ps -e8in -c Example.hp
```

It outputs:

`hp2ps: Example.hp, line 43, samples out of sequence`

Trac field | Value |
---|---|

Version | 8.2.1 |

Type | Bug |

TypeOfFailure | OtherFailure |

Priority | normal |

Resolution | Unresolved |

Component | Runtime System |

Test case | |

Differential revisions | |

BlockedBy | |

Related | #14006 |

Blocking | |

CC | |

Operating system | |

Architecture |

I've found that this error is caused by the order of the arguments to `tcMatchTy`

in the following code.

```
mkOneConFull x con = do
let res_ty = idType x
(univ_tvs, ex_tvs, eq_spec, thetas, _req_theta , arg_tys, con_res_ty)
= conLikeFullSig con
tc_args = tyConAppArgs res_ty
subst1 = case con of
RealDataCon {} -> zipTvSubst univ_tvs tc_args
PatSynCon {} -> expectJust "mkOneConFull" (tcMatchTy con_res_ty res_ty)
...
```

Adding some debugging information I noticed that the call that makes `tcMatchTy`

return `Nothing`

and therefore trigger the error it is done with the following arguments (as outputed with -dppr-debug):

```
con_res_ty = main:T14135.Foo{tc r9} ghc-prim:GHC.Types.Int{(w) tc 3u}
res_ty = main:T14135.Foo{tc r9} (a{tv aWz} [tv] :: *)
```

As far as I understand the problem stands because `tcMatchTy`

expects the first argument to be a kind of template type that will get instantiated to match the second argument.
However, it is clear that there is no substitution s such that s(Int) = a.

If we change the call to `tcMatchTy res_ty con_res_ty`

then the example program compiles fine but when trying to validate, ghc is not able to build anymore `Data.Typeable.Internal`

because it triggers the exactly same error.

I have found that by substituting `tcMatchTy`

by `tcUnifyTy`

we solve the full problem, however, I don't know if `tcMatchTy`

should be prefered over `tcUnify`

or not (if it makes a semantic difference).

Any insight on this?

Trac field | Value |
---|---|

Differential revisions | - → D3981 |

The FreeKiTyVars datatype is local to the RnTypes module, so I think the way to go is to change the OccSet for a RdrSet (with the same API) and include this one on RdrName.

The only difficulty I find is to come up with a sensible Uniquable instance for RdrName.

```
instance Uniquable RdrName where
getUnique (Exact name) = ...
getUnique (Unqual occ) = ...
getUnique (Qual mod occ) = ...
getUnique (Orig mod occ) = ...
```

In the cases for **Exact** and **Unqual** we would like to tag the *Unique* of name and occ and for the cases of **Qual** and **Orig** to tag and combine somehow the Unques for mod and occ.

How can we do this?

Trac field | Value |
---|---|

Differential revisions | - → D3641 |

After some debugging I found out that the bug arises in the following function from RnTypes

```
extract_hs_tv_bndrs :: [LHsTyVarBndr GhcPs] -> FreeKiTyVars
-> FreeKiTyVars -> RnM FreeKiTyVars
-- In (forall (a :: Maybe e). a -> b) we have
-- 'a' is bound by the forall
-- 'b' is a free type variable
-- 'e' is a free kind variable
extract_hs_tv_bndrs tvs
(FKTV acc_kvs acc_k_set acc_tvs acc_t_set acc_all)
-- Note accumulator comes first
(FKTV body_kvs body_k_set body_tvs body_t_set body_all)
| null tvs
= return $
FKTV (body_kvs ++ acc_kvs) (body_k_set `unionOccSets` acc_k_set)
(body_tvs ++ acc_tvs) (body_t_set `unionOccSets` acc_t_set)
(body_all ++ acc_all)
| otherwise
= do { FKTV bndr_kvs bndr_k_set _ _ _
<- foldrM extract_lkind emptyFKTV [k | L _ (KindedTyVar _ k) <- tvs]
; let locals = mkOccSet $ map (rdrNameOcc . hsLTyVarName) tvs
; return $
FKTV (filterOut ((`elemOccSet` locals) . rdrNameOcc . unLoc) (bndr_kvs ++ body_kvs) ++ acc_kvs)
((body_k_set `minusOccSet` locals) `unionOccSets` acc_k_set `unionOccSets` bndr_k_set)
(filterOut ((`elemOccSet` locals) . rdrNameOcc . unLoc) body_tvs ++ acc_tvs)
((body_t_set `minusOccSet` locals) `unionOccSets` acc_t_set)
(filterOut ((`elemOccSet` locals) . rdrNameOcc . unLoc) (bndr_kvs ++ body_all) ++ acc_all) }
```

In the case *tvs* is empty, the **locals** variable holds the set of OccName found in tvs. The OccName of variable *a2* in the program with the line:

`[f,a2] <- mapM newName ["f","a"]`

is 'a' and thus the variable is filtered out from *bndr_kvs ++ body_kvs* (Which I guess holds the variable 'a' from *data Maybe a*). However, in the program with the line:

`[f,a2] <- mapM newName ["f","b"]`

the *OccName* of *a2* is "b" and doesn't get filtered out.

In the previous version of the commit 67465497 that changed this function, the variables where filtered regarding the full name (Which in this case is a *Exact* name) not only its OccName and the problem didn't existed.

I guess one way to solve this would be to filter out again based on full names and not only the *OccName* of variables.

Any thoughts?

Looking into it at ZuriHac, we suspect the function dropWildCards is where the problem lies.

This bug seems to be solved in ghc version 8.1.20160519 (commit 296b8f1b).

Maybe we should change `rts/RtsFlags.c`

to make use of <getopt.h> because now the option parser is a bit scaring. I think I can do it myself. Any suggestions?

Trac field | Value |
---|---|

Differential revisions | → D748 |

I added this warning flag and updated the documentation according to it. The new flag is documented as follows:

-fwarn-unticked-promoted-constructors:

Warn if a promoted data constructor is used without a tick preceding it's name.

For example:

```
data Nat = Succ Nat | Zero
data Vec n s where
Nil :: Vec Zero a
Cons :: a -> Vec n a -> Vec (Succ n) a
```

This program will raise two warnings because Zero and Succ are not written as 'Zero and 'Succ.

This warning is off by default.

If anybody thinks this can be explained better, all suggestions are welcomed!

Trac field | Value |
---|---|

Test case | → T9778 |

Differential revisions | → D534 |

Trac field | Value |
---|---|

Test case | → T9776 |

Differential revisions | → D503 |