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

Nonblocking file unlock for virtiofs #6823

Closed
9 of 10 tasks
Eveldee opened this issue Apr 15, 2023 · 4 comments · Fixed by #6840
Closed
9 of 10 tasks

Nonblocking file unlock for virtiofs #6823

Eveldee opened this issue Apr 15, 2023 · 4 comments · Fixed by #6840
Labels
bug Bug that is not site-specific

Comments

@Eveldee
Copy link
Contributor

Eveldee commented Apr 15, 2023

DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE

  • I understand that I will be blocked if I intentionally remove or skip any mandatory* field

Checklist

  • I'm reporting a bug unrelated to a specific site
  • I've verified that I'm running yt-dlp version 2023.03.04 (update instructions) or later (specify commit)
  • I've checked that all provided URLs are playable in a browser with the same IP and same login details
  • I've checked that all URLs and arguments with special characters are properly quoted or escaped
  • I've searched known issues and the bugtracker for similar issues including closed ones. DO NOT post duplicates
  • I've read the guidelines for opening an issue

Provide a description that is worded well enough to be understood

Using some filesystems such as virtiofs, trying a to download a video will result in a error due to blocking locks not being supported in this filesystem. The lock part already has the non-blocking flag so there is no issue, only the unlocking part don't have the flag and is blocking.

Adding the flag LOCK_NB to the function _unlock_file(f) in utils.py fixes it but I don't know the implications it could have using other filesystems so I'm not sure if a pull request should be done for it.

def _unlock_file(f):
	try:
		fcntl.flock(f, fcntl.LOCK_UN | fcntl.LOCK_NB)
	except OSError:
		fcntl.lockf(f, fcntl.LOCK_UN | fcntl.LOCK_NB)

Provide verbose output that clearly demonstrates the problem

  • Run your yt-dlp command with -vU flag added (yt-dlp -vU <your command line>)
  • If using API, add 'verbose': True to YoutubeDL params instead
  • Copy the WHOLE output (starting with [debug] Command-line config) and insert it below

Complete Verbose Output

[debug] Command-line config: ['-Uv', 'https://www.youtube.com/watch?v=iNVv3EQ2MPs']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] yt-dlp version stable@2023.03.04 [392389b7d]
[debug] Python 3.11.3 (CPython x86_64 64bit) - Linux-6.1.23-0-virt-x86_64-with (OpenSSL 3.1.0 14 Mar 2023)
[debug] exe versions: ffmpeg 6.0 (setts), ffprobe 6.0
[debug] Optional libraries: Cryptodome-3.17, brotli-1.0.9, certifi-2022.12.07, mutagen-1.46.0, secretstorage-3.3.3, sqlite3-2.6.0, websockets-11.0.1
[debug] Proxy map: {}
[debug] Loaded 1786 extractors
[debug] Fetching release info: https://api.github.com/repos/yt-dlp/yt-dlp/releases/latest
Available version: stable@2023.03.04, Current version: stable@2023.03.04
yt-dlp is up to date (stable@2023.03.04)
[youtube] Extracting URL: https://www.youtube.com/watch?v=iNVv3EQ2MPs
[youtube] iNVv3EQ2MPs: Downloading webpage
[youtube] iNVv3EQ2MPs: Downloading android player API JSON
[debug] Sort order given by extractor: quality, res, fps, hdr:12, source, vcodec:vp9.2, channels, acodec, lang, proto
[debug] Formats sorted by: hasvid, ie_pref, quality, res, fps, hdr:12(7), source, vcodec:vp9.2(10), channels, acodec, lang, proto, filesize, fs_approx, tbr, vbr, abr, asr, vext, aext, hasaud, id
[debug] Default format spec: bestvideo*+bestaudio/best
[info] iNVv3EQ2MPs: Downloading 1 format(s): 248+251
[debug] Invoking dashsegments downloader on "https://rr1---sn-n4g-nmcd.googlevideo.com/videoplayback?expire=1681611495&ei=hwY7ZJy8D_u_mLAPo6aHuAQ&ip=2a02%3A8428%3A9552%3A4101%3A5054%3Aff%3Afe69%3Aa080&id=o-AJHa-NA6jUjkhfMjsAmGUEzERQgzC64Q8Bu5EKmiVG7T&itag=248&source=youtube&requiressl=yes&mh=vy&mm=31%2C29&mn=sn-n4g-nmcd%2Csn-n4g-jqbed&ms=au%2Crdu&mv=m&mvi=1&pl=47&initcwndbps=1263750&spc=99c5CUauX2DbtsvxSGDkbrMxCBgACuI&vprv=1&svpuc=1&mime=video%2Fwebm&gir=yes&clen=152064876&dur=562.033&lmt=1680386995166053&mt=1681589590&fvip=2&keepalive=yes&fexp=24007246&c=ANDROID&txp=6319224&sparams=expire%2Cei%2Cip%2Cid%2Citag%2Csource%2Crequiressl%2Cspc%2Cvprv%2Csvpuc%2Cmime%2Cgir%2Cclen%2Cdur%2Clmt&sig=AOq0QJ8wRQIgeA9REcTbUlZRBZs2Ur0qJe1ff2IddrcXDimTMswjStwCIQD52Bo0Kgc5t2eTsFZeEMk1DgI_OVyFkxrSfXpM02Y9UQ%3D%3D&lsparams=mh%2Cmm%2Cmn%2Cms%2Cmv%2Cmvi%2Cpl%2Cinitcwndbps&lsig=AG3C_xAwRQIgNCysydweXABclnBWGg7bbdM8p2bpZuZXmMuKFhGQfl8CIQCU2s6m9A56WZLQPYTecp0PjfPfg6iYIIlFAEul8i5ISg%3D%3D"
[dashsegments] Total fragments: 15
[download] Destination: A Complete Noob's FIRST Elden Ring Experience | Dragon, Bear and Thing Time! [iNVv3EQ2MPs].f248.webm
ERROR: unable to download video data: [Errno 95] Not supported
Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/yt_dlp/utils.py", line 2188, in _unlock_file
    fcntl.flock(f, fcntl.LOCK_UN)
OSError: [Errno 95] Not supported

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.11/site-packages/yt_dlp/YoutubeDL.py", line 3229, in process_info
    partial_success, real_download = self.dl(fname, new_info)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/yt_dlp/YoutubeDL.py", line 2970, in dl
    return fd.download(name, new_info, subtitle)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/yt_dlp/downloader/common.py", line 444, in download
    ret = self.real_download(filename, info_dict)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/yt_dlp/downloader/dash.py", line 41, in real_download
    self._prepare_and_start_frag_download(ctx, fmt)
  File "/usr/lib/python3.11/site-packages/yt_dlp/downloader/fragment.py", line 81, in _prepare_and_start_frag_download
    self._prepare_frag_download(ctx)
  File "/usr/lib/python3.11/site-packages/yt_dlp/downloader/fragment.py", line 205, in _prepare_frag_download
    self._write_ytdl_file(ctx)
  File "/usr/lib/python3.11/site-packages/yt_dlp/downloader/fragment.py", line 114, in _write_ytdl_file
    frag_index_stream.close()
  File "/usr/lib/python3.11/site-packages/yt_dlp/utils.py", line 2252, in __exit__
    self.unlock()
  File "/usr/lib/python3.11/site-packages/yt_dlp/utils.py", line 2246, in unlock
    _unlock_file(self.f)
  File "/usr/lib/python3.11/site-packages/yt_dlp/utils.py", line 2190, in _unlock_file
    fcntl.lockf(f, fcntl.LOCK_UN)
OSError: [Errno 95] Not supported
@Eveldee Eveldee added bug Bug that is not site-specific triage Untriaged issue labels Apr 15, 2023
@pukkandan
Copy link
Member

What does it even mean to unlock "non-blocking"? According to the docs LOCK_NB is not valid alongside LOCK_UN. That said, it does work fine on my system, but more testing will be needed. Alternatively, to be on the safe side, we could add NB only when a errno.ENOTSUP is raised

@pukkandan pukkandan removed the triage Untriaged issue label Apr 15, 2023
@Eveldee
Copy link
Contributor Author

Eveldee commented Apr 15, 2023

It indeed does not make much sense, It is probably an oversight in the code of virtiofsd where they check for the flag on any file lock related operation, even for file unlocking which maybe shouldn't be needed except if some systems do support it, I don't have much insight for that.

I think adding the flag only when an errno.ENOTSUP is raised is the way to go to make sure to not change any behavior for already well-functioning filesystems.

@pukkandan
Copy link
Member

Will you be opening a PR?

@Eveldee
Copy link
Contributor Author

Eveldee commented Apr 17, 2023

After more research I found that the flag LOCK_NB is indeed not supported by lockf while it is for flock.
So for virtiofs the correct call is fcntl.flock(f, fcntl.LOCK_UN | fcntl.LOCK_NB). I still added it in a try/catch just in case to make sure it doesn't break any already working configuration

pukkandan pushed a commit that referenced this issue May 5, 2023
Authored by: brandon-dacrib
Closes #6823
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this issue Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that is not site-specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants