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

Fix db deadlock when running multiple instances at once #26

Closed
wants to merge 1 commit into from
Closed

Fix db deadlock when running multiple instances at once #26

wants to merge 1 commit into from

Conversation

ThinkChaos
Copy link
Contributor

Hi,

I ran into #24 and the timeout fix is a bit hacky. Unfortunately I can't build the latest commit (when using vendoring), so I'm opening this PR targeting an older commit just to show you how this can be properly fixed.

Basically there's a race for locking the DB, so the solution is closing it earlier, and opening it later. More specifically, before writing to/after reading from stdout/stdin is done so the process earlier/later in the shell pipeline has exited (or at least unlocked).

I hope you can port this to the latest commit and make a new release.
Thanks!

@ThinkChaos ThinkChaos marked this pull request as ready for review March 12, 2022 03:00
@sentriz sentriz closed this in 4a54efe Mar 12, 2022
@sentriz
Copy link
Owner

sentriz commented Mar 12, 2022

thanks! makes sense since every command in the pipeline starts at once

@sentriz
Copy link
Owner

sentriz commented Mar 12, 2022

how does it work for you with this? https://github.com/sentriz/cliphist/releases/tag/v0.3.1

@manfred3000
Copy link

It's working flawlessly here, so far.
Thanks @sentriz @ThinkChaos

@ThinkChaos
Copy link
Contributor Author

Works for me too. Thanks for being so quick!

P.S. Figured out my issue with vendoring: was using Go 1.17 instead of 1.16

@sentriz
Copy link
Owner

sentriz commented Mar 12, 2022

nice! thanks for the patch :)

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 this pull request may close these issues.

3 participants