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

youtube-dl doesn't use locking to prevent concurrent writing to output files #10336

Closed
ssokolow opened this issue Aug 13, 2016 · 2 comments
Closed

Comments

@ssokolow
Copy link

@ssokolow ssokolow commented Aug 13, 2016

  • I've verified and I assure that I'm running youtube-dl 2016.08.12
  • At least skimmed through README and most notably FAQ and BUGS sections
  • Searched the bugtracker for similar issues including closed ones

What is the purpose of your issue?

  • Bug report (encountered problems with youtube-dl)
  • Site support request (request for adding support for a new site)
  • Feature request (request for a new functionality)
  • Question
  • Other

If the purpose of this issue is a bug report, site support request or you are not completely sure provide the full verbose output as follows:

% youtube-dl -v
[debug] System config: []
[debug] User config: []
[debug] Command-line args: [u'-v']
[debug] Encodings: locale UTF-8, fs UTF-8, out UTF-8, pref UTF-8
[debug] youtube-dl version 2016.08.12
[debug] Python version 2.7.12 - Linux-3.13.0-91-generic-x86_64-with-Ubuntu-14.04-trusty
[debug] exe versions: avconv 2.8.1-1, avprobe 2.8.1-1, ffmpeg 2.8.1-1, ffprobe 2.8.1-1, rtmpdump 2.4
[debug] Proxy map: {}
Usage: youtube-dl [OPTIONS] URL [URL...]

youtube-dl: error: You must provide at least one URL.
Type youtube-dl --help to see a list of all options.

It doesn't matter which URLs it's being run on and I couldn't find any evidence of concurrency-aware operation in the --help output or FAQ, so I didn't think it was necessary to get verbose output from the actual operation.


Description of your issue, suggested solution and other information

I run youtube-dl via a cronscript to download new videos on certain YouTube channels for offline viewing. However, I sometimes also run the script manually. This can result in two copies of youtube-dl trying to download the same URL into the same folder at the same time.

When that happens, the interactions between the two copies of youtube-dl cause the tail end of the video to be repeated in the file. youtube-dl should use some form of locking to prevent this.

Given the flaws in all of the more portable approaches to locking (eg. lock files can be stale, sockets aren't scoped to specific files, etc.), I'd suggest checking os.name and then using either POSIX-specific or Windows-specific locking functionality.

For example, for POSIX OSes, this is one of the few situations where the flawed fcntl-based locking system would work well.

# `fcntl` is a stdlib module

do_download = True
if os.name == 'posix':
    # fcntl(2): to place a write lock, fd must be open for writing.
    lock_fobj = open(target_path, 'wb')   
    try:
        fcntl.lockf(lock_fobj, fcntl.LOCK_EX | fcntl.LOCK_NB)
    except IOError:
        log.warning("Download already in progress in another process: %s", target_path)
        do_download = False

if do_download:
    # <normal youtube-dl stuff>

if os.name == 'posix':
    lock_fobj.close()  
    # fcntl(2): record locks are automatically released when the process terminates or
    #  if it closes **any** file descriptor referring to a file on which locks are held.

# In fact, this'd be easy to nicely encapsulate in a context manager so you
# could just assign `lockf_for_this_platform` and 
# then go `with lockf_for_this_platform(target_path):`

POSIX fcntl locks are advisory (they only affect future attempts to call fcntl.lockf) and you're clearly not opening your files for exclusive access (good, because it allows me to play the *.part files as long as they download faster than they play), so this won't interfere with existing code in any way.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Aug 13, 2016

There is already a class locked_file in youtube_dl/utils.py providing file-locking functionality. Currently only download archives are locked. I'm not sure why other files are not locked, though.

A side note for developers, if someone is going to implement it: Jython does not have fcntl module, so files are not locked on Jython. (Jython is not officially supported. I just implement basic Jython support since #8302)

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Aug 13, 2016

Found an old thread discussing similar scenarios. Closing this and leaving further discussion to #485

@yan12125 yan12125 closed this Aug 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.