Bug #37950 for MP3-Tag: id3v2 TMPXX temp file problem when multi=threaded

Hello Ilya, Thanks for the response & review of my submission. Threads seemed natural for me to use. My program scans a number of directories and plyalist files, then creates a number of new directories in another location with mp3 files transcoded to a lower rate to occupy less space. It's done on windows. The main program figures out the list, then uses a pool of threads to walk through converting the files. I suppose I could use processes for each conversion, but then I'd need to write command line handling on my main program to understand each operation & pass the relevant data for each operation. I'd also guess that overhead for a new process would be >> threads. Threads felt a bit more natural as I could pass perl data structures to the threads and call specific perl subroutines more directly ... I hadn't thought of going the process route until you mentioned it - thanks for the idea - it may be the only solution to another problem I ran into - the same program uses' OLE to talk with itunes, and that causes fatal errors with threads. The error I saw with MP3::Tag is the only threaded one I've seen so far - baring any major glaring bugs on my side - this implies your package is very close to being thread safe. When I used the word "this" below, I meant adding the new perl line "my $tempfile = $mp3obj->{filename} . "_TMPxx"; didn't seem to solve the problem, though I expected it to do so. My script prints out the name of the file it is modifying [from each thread] just before it copies the mp3 tags, and I haven't seen the same filename crop up twice, so I don't think that's the problem. Good question! Since the temp file does exist, could the source be some other error, perhaps a system call attempting to run the move command fails for some other reason? Thanks, Rob On Wed, Jul 30, 2008 at 3:31 PM, Ilya Zakharevich via RT < bug-MP3-Tag@rt.cpan.org> wrote: Show quoted text
-- Cheers, Rob Laddish, rob@laddish.net

Hello, Eight years after the initial report, I too have encountered this problem. However not multi-threaded like the original bug reporter, but rather multi-process (possibly single process, see below). I'm using a tool known as ‘abcde’¹ to rip CD's, combined with a custom wrapper that emulates the command-line syntax of ‘id3v2’, which I've written in Perl using MP3::Tag. Frequently, during an abcde rip, the abcde script will fail on one or two tracks during it's “tag” operation. Fortunately abcde can be executed again, resuming the failed operations and completing the rip. It's quite difficult to chase down the cause until you make use of the abcde's LOWDISK environment variable so that messages from external tools, like my script based on MP3::Tag, don't have their output sent to /dev/null. abcde does parallelize some operations (particularly encoding), however with the LOWDISK option set, I believe everything is done sequentially. FWIW I am using Linux (pre-release of Debian stretch, kernel 4.6, Perl 5.22), on a btrfs filesystem, on an SSD. The following is the output when my script using MP3::Tag fails during an abcde rip: mv: cannot stat ‘/home/gturner/mp3/abcde.fd123b11/TMPxx1.tmp’: No such file or directory Couldn't rename temporary file /home/gturner/mp3/abcde.fd123b11/TMPxx1.tmp to /home/gturner/mp3/abcde.fd123b11/track15.mp3 Uncaught exception from user code: Failed to write ID3v2 tag to file "/home/gturner/mp3/abcde.fd123b11/track15.mp3" at /home/gturner/bin/abcde-id3v2-wrapper line 210. MP3ArchiveScripts::update_frames("/home/gturner/mp3/abcde.fd123b11/track15.mp3", ARRAY(0xc81e88)) called at /home/gturner/bin/abcde-id3v2-wrapper line 210 [ERROR] abcde: The following commands failed to run: tagtrack-mp3-15: returned code 2: nice -n 10 abcde-id3v2-wrapper -c -A Nature Mortes - Still Lives -a Sort Sol -t Misguided -y 1981 -g 255 -T 15/17 --TPE2 Various /home/gturner/mp3/abcde.fd123b11/track15.mp3 The following is a portion of the code from MP3::Tag::ID3v2 insert_space funciton: # insert_space() copies a mp3-file and can insert one or several areas # of free space for a tag. These areas are defined as # ($pos, $old_size, $new_size) # $pos says at which position of the mp3-file the space should be inserted # new_size gives the size of the space to insert and old_size can be used # to skip this size in the mp3-file (e.g if sub insert_space { my ($self, $insert) = @_; my $mp3obj = $self->{mp3}; # !! use a specific tmp-dir here my $tempfile = dirname($mp3obj->{filename}) . "/TMPxx"; my $count = 0; while (-e $tempfile . $count . ".tmp") { if ($count++ > 999) { warn "Problems with tempfile\n"; return undef; } } $tempfile .= $count . ".tmp"; unless (open (NEW, ">$tempfile")) { warn "Can't open '$tempfile' to insert tag\n"; return -1; } my ($buf, $pos_old); binmode NEW; … snip … close NEW; $mp3obj->close; # rename tmp-file to orig file unless (( rename $tempfile, $mp3obj->{filename})|| (system("mv",$tempfile,$mp3obj->{filename})==0)) { unlink($tempfile); warn "Couldn't rename temporary file $tempfile to $mp3obj->{filename}\n"; return -1; } return 0; } There's a race condition between the routine checking for the absence of a deterministic (non-random) filename, and the routine deciding to use that filename. I modified my script to move files to their own temporary directory before invoking MP3::Tag->new and these failures have gone away. I'd be happy to create a patch that uses core module File::Temp and perhaps File::Copy. However if there isn't any interest, I'll just carry on with my work-around. Please let me know. ¹ https://abcde.einval.com/wiki/ -- Gerald Turner <gturner@unzane.com> Encrypted mail preferred! OpenPGP: 4096R / CA89 B27A 30FA 66C5 1B80 3858 EC94 2276 FDB8 716D