NonDet handler throws storage assertion

[michaelpj]

Env and LocalEnv point to different Storages.
If you passed LocalEnv to a different thread and tried to create an unlifting function there, it's not allowed. You need to create it in the thread of the effect handler.
CallStack (from HasCallStack):
 error, called at src/Effectful/Dispatch/Dynamic.hs:1114:40 in effectful-core-2.4.0.0-1Y97Z0HsCQhKZtcRsL9K4V:Effectful.Dispatch.Dynamic
 requireMatchingStorages, called at src/Effectful/Dispatch/Dynamic.hs:750:3 in effectful-core-2.4.0.0-1Y97Z0HsCQhKZtcRsL9K4V:Effectful.Dispatch.Dynamic
 localSeqUnlift, called at src/Effectful/NonDet.hs:87:5 in effectful-core-2.4.0.0-1Y97Z0HsCQhKZtcRsL9K4V:Effectful.NonDet
 handler, called at src/Effectful/Dispatch/Dynamic.hs:461:77 in effectful-core-2.4.0.0-1Y97Z0HsCQhKZtcRsL9K4V:Effectful.Dispatch.Dynamic
 send, called at src/Effectful/Internal/Monad.hs:302:13 in effectful-core-2.4.0.0-1Y97Z0HsCQhKZtcRsL9K4V:Effectful.Internal.Monad

I don't have a small reproducer, but when trying to upgrade to 2.4 we see this error. It doesn't look like it's anything to do with our code, but rather something to do with NonDet. The unlifting function in question is created inside the handler of NonDet, and is then only called within that handler. But there is some shennanigans with backing up and restoring environments, perhaps that can trigger this?

[ocharles]

We're upgrading from 1f1f351, if that helps!

[arybczak]

Can you give me the code of your handler?

[michaelpj]

I'm not sure which bit you want? We're not handling NonDet, that's the handler defined in effectful that's throwing.

[arybczak]

I'm not sure which bit you want? We're not handling NonDet, that's the handler defined in effectful that's throwing.

Right. That's what I get for reading stuff on my phone 😅

Is this the same place #243 throws in? You mentioned there that the bug disappeared when you switched to NonDetKeep, but this is NonDetRollback. Did you switch back?

[arybczak]

FWIW I think it's the same thing as #243, just manifesting earlier due to the new sanity check.

[arybczak]

Btw, are you using interpose or impose anywhere?

[ocharles]

Nope, we don't use either (well, we have one use of impose in CircuitHub, but not in this project or any of its deps)

[michaelpj]

You mentioned there that the bug disappeared when you switched to NonDetKeep, but this is NonDetRollback. Did you switch back?

No, it's in a different place where apparently I decided to use OnEmptyRollback for some reason... I can see if it goes away if we turn it into OnEmptyKeep, which I think will be safe.

[arybczak]

Btw, the call stack from the first message is relatively useless because it ends inside the call to <|>. I added df06ee3 which adds plusEff which if used instead of <|> will give useful call stack with the location in your code.

Is this problem at least reliably reproducible for you now that the new sanity check is in place?

If so, I'd like to see the surrounding code of the offending call site. Generally speaking I need to see something. I need a reproducer, doesn't have to be small, because so far me playing a playing a guessing game unfortunately hasn't got us anywhere near the solution. effectful gives much better call stacks now than before, but that's it 😞

I'd like to get to the bottom of this because I find this problem concerning, but I can't do much if you don't show me any code.

[michaelpj]

That's totally fair, as usual I thought it was worth putting up what we have now in case it happened to tickle your brain into figuring it out :) I'll investigate more.

[arybczak]

Ok, I think I got a reproducer.

I did this:

diff --git a/effectful-core/src/Effectful/Internal/Monad.hs b/effectful-core/src/Effectful/Internal/Monad.hs
index 1ac5f79..d1822bb 100644
--- a/effectful-core/src/Effectful/Internal/Monad.hs
+++ b/effectful-core/src/Effectful/Internal/Monad.hs
@@ -553,6 +553,8 @@ send
   -> Eff es a
 send op = unsafeEff $ \es -> do
   Handler handlerEs handler <- getEnv es
+  when (envStorage handlerEs /= envStorage es) $ do
+    error "wtf"
   -- Prevent internal functions that rebind the effect handler from polluting
   -- its call stack by freezing it. Note that functions 'interpret',
   -- 'reinterpret', 'interpose' and 'impose' need to thaw it so that useful

And I'm getting wtfs in NonDet/OnEmptyRollback tests. I'll have a look tomorrow.

[arybczak]

Can you check #256? It should fix this.

[michaelpj]

Confirmed that this fixes it!

[ocharles]

@arybczak Do you plan to release a minor version with this bug fix, or should we just run with HEAD?

[arybczak]

You can run with HEAD for a moment, I plan to merge the overhaul of Effectful.Exception and release 2.5.0.0 in a week or so.