New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Control.Concurrent.STM.atomically was nested #86

Closed
mitchellwrosen opened this Issue Jun 14, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@mitchellwrosen
Contributor

mitchellwrosen commented Jun 14, 2016

foo.hs:

{-# LANGUAGE OverloadedStrings #-}

import Control.Concurrent.STM
import Data.Yaml

main = do
  atomically (let x = decode "" :: Maybe Value in x `seq` x)
  pure ()

result:

$ stack runghc --resolver lts-6.3 foo.hs
foo.hs: Control.Concurrent.STM.atomically was nested

Uh...? I don't believe this is actually a yaml bug because it doesn't depend on stm, but one of its dependencies, somewhere, is calling some unsafePerformIO (atomically foo)-type thing. Not too sure where to look from here.

@mitchellwrosen

This comment has been minimized.

Contributor

mitchellwrosen commented Jun 14, 2016

Hmm... I looked into this a bit more. I think it's the tryAny in Data.Yaml.Internal.decodeHelper_, which uses waitCatch, inside of decode's unsafePerformIO. Plus, you know, laziness and stuff.

@mitchellwrosen

This comment has been minimized.

Contributor

mitchellwrosen commented Jun 14, 2016

I'm guessing rewriting decode to not use unsafePerformIO would be a big undertaking, so I think some kind of comment disclaimer saying "do not use decode inside of an STM computation" would be sufficient.

@snoyberg

This comment has been minimized.

Owner

snoyberg commented Jun 15, 2016

Actually, there's probably a better solution available: modify tryAny to not use the async package, and instead of using TVars use MVars for the synchronization, which have no nesting issue. Are you interested in sending a PR to enclosed-exception to do that?

@mitchellwrosen

This comment has been minimized.

Contributor

mitchellwrosen commented Jun 15, 2016

Sure, I can give it a go.
On Jun 15, 2016 1:01 AM, "Michael Snoyman" notifications@github.com wrote:

Actually, there's probably a better solution available: modify tryAny to
not use the async package, and instead of using TVars use MVars for the
synchronization, which have no nesting issue. Are you interested in sending
a PR to enclosed-exception to do that?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#86 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABBlpnWR0yiLV54c8MXbJ8bn2leijZQgks5qL4cmgaJpZM4I1lj6
.

@mitchellwrosen

This comment has been minimized.

Contributor

mitchellwrosen commented Jun 15, 2016

@snoyberg

This comment has been minimized.

Owner

snoyberg commented Jun 22, 2016

Can you confirm that this issue does not exist with the newly released enclosed-exceptions version?

@snoyberg

This comment has been minimized.

Owner

snoyberg commented Aug 31, 2016

Closing as believed to be fixed.

@snoyberg snoyberg closed this Aug 31, 2016

aminb added a commit to unitb/literate-unitb-complete that referenced this issue Aug 18, 2017

Pin yaml to >= 0.8.22 to avoid issue with STM
yaml < 0.8.22 causes literate-unitb-logic's test suite to throw an
exception: Control.Concurrent.STM.atomically was nested

Possibly related: snoyberg/yaml#86

Not sure about the exact cause, but we don't seem to have the issue
with yaml >= 0.8.22, so I'm bumping the lower bound as a workaround.

cipher1024 added a commit to unitb/literate-unitb-complete that referenced this issue Aug 24, 2017

unitb-config revamp (#34)
Swap out ConfigFile for yaml

* [1/4] Swap out ConfigFile for yaml
Config file's name changed from z3_config.conf to z3_config.yml.

TODO: there's a slight issue with the yaml library treating all
of Z3Config's fields as String, in that it will require the numbers
to be explicitly quoted to be considered as string literals; and
otherwise it will ignore them. We'll try to fix this by generalizing
the Document type class and the functions using it.

* [2/4][WIP] Generalize Document

* [3/4] Generalize Document (cleanup)

- literate-unitb-config compiles again
- Add traverseOf (similar to lensOf and prismOf)
- Swap `fieldWith`'s arguments to make them consistent with `field`

* [4/4] Rename unitb-option executable to unitb-config

* Pin yaml to >= 0.8.22 to avoid issue with STM

yaml < 0.8.22 causes literate-unitb-logic's test suite to throw an
exception: Control.Concurrent.STM.atomically was nested

Possibly related: snoyberg/yaml#86

Not sure about the exact cause, but we don't seem to have the issue
with yaml >= 0.8.22, so I'm bumping the lower bound as a workaround.

* Clean up comments

* Align =>, ->, and :: properly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment