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

Concurrent add and pin doesn't work #5376

Closed
Stebalien opened this issue Aug 13, 2018 · 6 comments
Closed

Concurrent add and pin doesn't work #5376

Stebalien opened this issue Aug 13, 2018 · 6 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@Stebalien
Copy link
Member

... unless ipfs add --pin=false is specified. It looks like we're taking the pin lock too early somewhere.

@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Aug 13, 2018
@Stebalien
Copy link
Member Author

This is not new. It has been an issue since before 0.4.13.

It looks like ipfs pin add takes a write lock on the pinset before starting the pin.

@Stebalien
Copy link
Member Author

Wow, the pinner doesn't even have access to the pin lock.

@bonedaddy
Copy link
Contributor

Let me know if there's anything I can do to help with this

@bonedaddy
Copy link
Contributor

bonedaddy commented Aug 15, 2018

@Stebalien Currently in my system I have a queue that process IPFS pin requests, and a queue that processes IPFS file add requests, which is how I initially saw this issue occur, when one queue was attempting to add a file, and the other was attempting to pin something else

Since the issue occurs will attempting to add a file with a pin, while also pinning; would a reasonable solution be to add files with no pin, then have a seperate process pin the content to my IPFS node?

@Stebalien
Copy link
Member Author

Test case:

ipfs dag put <<<'{"link": {"/": "QmdZmoS1b1jp9vL5vgTyebEYUVH6zjd6erhp3fpqV1Zktx"}}'
ipfs pin add zdpuAsWv1LNwtxvFB3iTQjtwDLxPxRaa3WeE4i6BoDuKDy4gf &
echo "thing" | ipfs add

The last two commands should block indefinitely.

@Stebalien
Copy link
Member Author

So, it turns out that we simply have a hidden requirement here: We need to take the GCLock before taking the pin lock.

Actually, after thinking about this, we can't even take this lock from within pinner.Pin as the caller may be holding it.

@ghost ghost removed the status/in-progress In progress label Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

3 participants