Release lock on Web Stream Reader after done parsing

[georgevarghese185]

Is there an existing issue for this?

• I have searched the existing issues

Feature description

I was having trouble trying to close my ReadableStream after getting the metadata I needed. I wanted to dispose of it ASAP to clear any resources being held.

But calling ReadableStream.close() threw an error because the library was still holding onto a reader somewhere. (Inside strtok3 or peek-readable packages I presume)

const meta = await parseWebStream(
  myReadableStream,
  { mimeType: 'audio/mpeg', path: filePath, size: fileSize },
  { skipCovers: true, skipPostHeaders: true },
);
await myReadableStream.close(); // Throws Error: Invalid state: ReadableStream is locked

I can solve this problem myself by using strtok3's fromWebStream() function to get a Tokenizer and then call Tokenizer.abort() as soon as parseTokenizer() returns with the metadata.

const tokenizer = fromWebStream(myReadableStream, { mimeType: 'audio/mpeg', path: filePath, size: fileSize });
const meta = await parseFromTokenizer(
  tokenizer,
  { skipCovers: true, skipPostHeaders: true },
);
await tokenizer.abort(); // Successfully closes the stream

But still, I was just wondering if it would make sense to support some kind of mechanism where the library calls Reader.releaseLock() when it's done parsing all the metadata it needs.

That would allow the supplier of the ReadableStream to do whatever they want with the stream after the metadata has been retrieved. They can choose to continue reading data (if it has more to read) or they can close the stream without any errors because the lock is not being held.

Doing this would probably require changes in the strtok3 and peek-readable packages as well. So I understand it might be complicated. It would need to expose a function similar to abort() but softer.. something that can release the lock on the stream's reader. So maybe something like release() or cleanup()? And this functionality wouldn't have to be implemented by all sources.. only those that have resource locking like Web streams.

Do you have any thoughts on this? I'd be glad to lend a hand with some PRs across the packages if you feel like it makes sense.

[Borewit]

Good catch @georgevarghese185. To me it looks like a bug in music-metadata. There is a tokenizer.close() method which is not called, in that shall release the stream it reading from.

Tokenizer is created, but never closed:

music-metadata/lib/core.ts

Line 43 in 91408ba

return parseFromTokenizer(fromWebStream(webStream, {fileInfo: typeof fileInfo === 'string' ? {mimeType: fileInfo} : fileInfo}), options);

[Borewit]

Let me know if it now behaving as expected @georgevarghese185 , or if there is still room for improvement.

[georgevarghese185]

Thanks @Borewit ! I've tested it a little bit and it seems to be working fine! I'll come back to this thread if I run into any related issues later