Skip to content
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

runInBoundThread considered harmful #6

Open
infinity0 opened this issue Jun 17, 2019 · 3 comments · May be fixed by #7
Open

runInBoundThread considered harmful #6

infinity0 opened this issue Jun 17, 2019 · 3 comments · May be fixed by #7

Comments

@infinity0
Copy link

infinity0 commented Jun 17, 2019

runInBoundThread forks off another thread. However, suppose you're writing an application that manages a bunch of threads. Due to POSIX standards [1] one has to code the main function to catch all exceptions, track child threads, cancel the child threads when you get an exception (which may trigger cleanups in each thread) and then wait for these child threads to exit.

runInBoundThread interferes with this process, since it (a) forks off a worker-bound-thread that nobody else knows about, (b) is uninterruptible while this worker-bound-thread runs, and (c) and if it receives an asynchronous exception in the meantime, it will not rethrow this to the worker-bound-thread. (Arguably the combination of (b) and (c) are design bugs and I'll file a GHC bug shortly.)

In the case of simple-lmdb this is problematic because the worker-bound-thread is often trying to take a lock e.g. in mdb_txn_begin where it calls _lockEnv. If the database has already been closed, then the lock is already taken. Then, the thread will hang forever because it is running inside of runInBoundThread which is uninterruptible.

The simple fix is to use withAsyncBound and wait from Control.Concurrent.Async - define runInBoundThread' action = withAsyncBound action wait and use this instead of runInBoundThread everywhere.

[1] these standards dictate that when the main-thread exits, this also causes all child threads to exit immediately without giving the Haskell runtime a chance to run cleanups

@infinity0 infinity0 linked a pull request Jun 17, 2019 that will close this issue
@infinity0
Copy link
Author

@infinity0
Copy link
Author

BTW, whether it's legitimate to close the database and then try to perform actions on it, is a separate discussion. My point is that, trying to perform subsequent actions should not result in an indefinite uninterruptible hang, as is the case with the current code. Generally close functions are idempotent or at the very least throw an immediate exception.

@dmbarbour
Copy link

Using the LMDB database after closing it will normally result in undefined behavior, likely a crash. An indefinite hang is (arguably) much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants