fix: deadlock while racing ipfs dag import and ipfs repo gc

[Jorropo]

This fixes a deadlock introduced in 1457b4f.

We can't use the coreapi here because it will try to take the PinLock (RLock) again, so revert this small part of 1457b4f.

This used cause a deadlock when concurrently running ipfs dag import concurrently with the GC.

The bug is that ipfs dag import takes an RLock with the PinLock. the cars are imported, leaving a wide window of time Then GC Takes a Lock on that same RWMutex while taking the GC Lock (it blocks because it waits for the RLock to be released). Then the car imports are finished and ipfs dag import tries to aqcuire the PinLock (doing an RLock) again in Api().Pin.

However at this point the RWMutex is starved, the runtime put a fence in front of RLocks if a Lock has been waiting for too lock (else you could have an endless stream of RLock / RUnlock forever delaying a Lock to ever go through).

The issue is that ipfs dag import's original RLock which is blocking everyone will be released once it returns, which only happens when Api().Pin completes.

So we have a deadlock (ABA kind ?), because ipfs dag import waits on the GC Lock, which waits on ipfs dag import.

Calling the Pinner directly does not acquire the PinLock again, and thus does not have this issue.

[MichaelMure]

Make sense to me as a fix.
Imho though, the real way would be to change CoreApi to allow more control over those things. The current API is quite limiting.

[Jorropo]

Imho though, the real way would be to change CoreApi to allow more control over those things. The current API is quite limiting.

Without seeing code I think this is a bad idea, how will that manifest takeLock bool argument ?

I think it make sense to have multiple APIs for internal and external things.

There very well could be some better solution I'm missing here tho.

[MichaelMure]

@Jorropo alternatively, that dag/import code should be integrated inside the CoreApi, kinda like unixfs/adder ? From a quick code search, it seems that dag/import is the only one that stands out outside of the CoreApi perimeter while acting on the pinner (ignoring things like initializeIpnsKeyspace).

Fixing that would allow using properly the internal APIs, take the lock properly ... and let me override pinner and blockstore properly :-D

[Jorropo]

Make sense, make it take a []io.Reader or some non go-cmds-ipfs iterable interface instead, feel free to send a PR (I would wait until ipfs/boxo#220 and #9746 are merged tho).