This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 24576 - Calling URL.createObjectURL() on a closed Blob
Summary: Calling URL.createObjectURL() on a closed Blob
Status: RESOLVED FIXED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: File API (show other bugs)
Version: unspecified
Hardware: PC Windows NT
: P2 normal
Target Milestone: ---
Assignee: Arun
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 25081
  Show dependency treegraph
 
Reported: 2014-02-07 09:54 UTC by sof
Modified: 2014-04-02 21:15 UTC (History)
4 users (show)

See Also:


Attachments

Description sof 2014-02-07 09:54:15 UTC
What is the required behavior for

 var blob = new Blob(); blob.close(); URL.createObjectURL(blob);

Failure (what exception, if so?), or observably equal to

 var blob = new Blob(); URL.createObjectURL(blob);

http://dev.w3.org/2006/webapi/FileAPI/#close-method says 

  "..once close() has been called on a Blob, that Blob cannot be used again."

unclear what that means exactly in this context. A clarification would be appreciated.
Comment 1 Anne 2014-02-07 09:59:34 UTC
It would be the same as

var blob = new Blob(); URL.createObjectURL(blob); blob.close();

But yes, the specification needs to describe its underlying model much more clearly. Arun will fix that in due course.
Comment 2 Simon Pieters 2014-02-07 10:58:46 UTC
I think close() should only set a flag on the Blob and then the relevant algorithms that need to care about it look at that flag (getting size, dereferencing blob: URL, whatever else). That would be clearer than the current requirement for close().
Comment 3 Arun 2014-02-07 15:35:32 UTC
(In reply to Simon Pieters from comment #2)
> I think close() should only set a flag on the Blob and then the relevant
> algorithms that need to care about it look at that flag (getting size,
> dereferencing blob: URL, whatever else). That would be clearer than the
> current requirement for close().

This sounds right to me, and is how I've begun to specify it. I think the flag should be an internal state keeper, and shouldn't really be an additional property on the Blob object.

A Blob that's been neutered with a close() call has a state of CLOSED; otherwise OPENED (or something similar). Then:

1. Both synchronous and asynchronous "read" will check for this state; the FileReader[Sync].readAsXXX will do the appropriate thing (error event or throw).

2. Blob.close() will also force the same sequence of steps as URL.revokeObjectURL(URL); essentially, a CLOSED Blob will be expunged from the Blob URL Store, and subsequent URL Parse will fail.  A question remains about whether asynchronous fetch actions should *also* fail, but I don't think so right now.
Comment 4 Arun 2014-02-27 22:36:42 UTC
SimonP, I think this is fixed. Can you verify this before I close the bug?

The relevant changes are:

http://dev.w3.org/2006/webapi/FileAPI/#close-method

and

http://dev.w3.org/2006/webapi/FileAPI/#createRevokeMethodsParams
Comment 5 Simon Pieters 2014-02-28 14:10:26 UTC
Did close() throw before for already-closed blobs?
Comment 6 sof 2014-02-28 14:12:58 UTC
Previous spec version, http://www.w3.org/TR/2012/WD-FileAPI-20121025/#close-method
Comment 7 Simon Pieters 2014-02-28 14:23:38 UTC
close() says to "Set the size of the context object to 0." but then reading size also says "If the Blob has a readability state of CLOSED then size must return 0." -- one of those seems unnecessary.

It's not clear to me what happens when an async read is started and the blob is closed in the same script. (Seems like it would fail to find the byte sequence since it is removed before the read starts?)

Do we want createObjectURL(closedBlob) to throw? Comment 1 seems to suggest it shouldn't throw.
Comment 8 Arun 2014-02-28 18:08:09 UTC
(In reply to Simon Pieters from comment #5)
> Did close() throw before for already-closed blobs?

No; this was based on discussions in IRC, and is a change. Do you disagree? If so, why?
Comment 9 Arun 2014-02-28 18:13:57 UTC
(In reply to Simon Pieters from comment #7)
> close() says to "Set the size of the context object to 0." but then reading
> size also says "If the Blob has a readability state of CLOSED then size must
> return 0." -- one of those seems unnecessary.


It's not overlapping redundancy right now (e.g. one doesn't overwrite the other), but I can change this to be in one place :-)


> 
> It's not clear to me what happens when an async read is started and the blob
> is closed in the same script. (Seems like it would fail to find the byte
> sequence since it is removed before the read starts?)


What should happen here IMHO is that if the read is underway, close() shouldn't have an effect. Of course, subsequent async reads will fail.

This should be made clearer. I don't think we can/should disrupt a read that's underway. Do you agree?

> 
> Do we want createObjectURL(closedBlob) to throw? Comment 1 seems to suggest
> it shouldn't throw.

This should not throw:

var blob = new Blob(); URL.createObjectURL(blob); blob.close();
(This is a non-neutered 0 bytes Blob).

This should throw:

var blob = new Blob(); blob.close(); URL.createObjectURL(blob);


I'm not sure I agree with Comment 1, but would appreciate input from Anne.
Comment 10 Anne 2014-03-17 18:20:21 UTC
I see why you ended up with the synchronous closed check now.

Man, allowing a blob to be closed really makes things harder. Basically anything that takes a blob needs to account for this somehow. 

In case of FormData it would be its rather complicated serialization algorithm... Which might end up doing this for several blobs in a row and then one of those could be closed, but we cannot start throwing at that point. Which would mean we would need some kind of lock upfront to prevent them from being closed as we want to read them in a bit and not throw. How much has this been thought through? Am I missing something obvious?


As for your scenario, you are saying that if you have a blob URL before the blob is closed, you can read it? The read operation algorithm does not seem to support that.
Comment 11 Simon Pieters 2014-03-18 12:36:49 UTC
(In reply to Arun from comment #8)
> No; this was based on discussions in IRC, and is a change.

OK. Do you remember the rationale (or is it logged)?

> Do you disagree?
> If so, why?

I would expect close() on a closed blob to be a no-op. I don't see much value in throwing. If you don't know if a blob is closed or not but want to close it, it seems like you would have to wrap it in a try/catch block. What is the benefit?

Possibly relevant is WebSocket#close() which is no-op if the socket is closed.
Comment 12 Simon Pieters 2014-03-18 12:51:22 UTC
(In reply to Arun from comment #9)
> What should happen here IMHO is that if the read is underway, close()
> shouldn't have an effect. Of course, subsequent async reads will fail.
> 
> This should be made clearer. I don't think we can/should disrupt a read
> that's underway. Do you agree?

That sounds good to me. The spec seems clearer now than when I wrote the earlier comment, IIRC.

> This should throw:
> 
> var blob = new Blob(); blob.close(); URL.createObjectURL(blob);

Why?
Comment 13 Arun 2014-03-19 14:58:09 UTC
(In reply to Simon Pieters from comment #12)
> (In reply to Arun from comment #9)
> > What should happen here IMHO is that if the read is underway, close()
> > shouldn't have an effect. Of course, subsequent async reads will fail.
> > 
> > This should be made clearer. I don't think we can/should disrupt a read
> > that's underway. Do you agree?
> 
> That sounds good to me. The spec seems clearer now than when I wrote the
> earlier comment, IIRC.


OK; a separate bug to either make the read operation fully asynchronous or synchronous with a wrapper is Bug 25081 and depending on how that shakes out, this could change slightly.

 
> 
> > This should throw:
> > 
> > var blob = new Blob(); blob.close(); URL.createObjectURL(blob);
> 
> Why?


Well, the reasoning is that read operations throw when you try to read a closed Blob. I think this seems about right. So, minting a blob URL on a closed blob should also throw, since that's a type of read but by another name (Parse/Fetch -- we should fail them aggressively right out of the starter gate, rather than have unpredictable Parse/Fetch behavior).

Alternatively, we could specify that it doesn't throw, but instead returns a blob URL that can be used for Parse/Fetch on 0 bytes. BUT:

var blob = new Blob();
var url = URL.createObjectURL(blob);

*also* returns a URL that can be used to Parse/Fetch on 0 bytes.

The difference is that this by design; calling blob.close(), however, is a programmatic declaration that the Blob should be closed and memory should be culled.
Comment 14 Simon Pieters 2014-03-19 15:34:58 UTC
(In reply to Arun from comment #13)
> Well, the reasoning is that read operations throw when you try to read a
> closed Blob. I think this seems about right. So, minting a blob URL on a
> closed blob should also throw, since that's a type of read but by another
> name (Parse/Fetch -- we should fail them aggressively right out of the
> starter gate, rather than have unpredictable Parse/Fetch behavior).

OK... I don't follow what would be unpredictable, though.

> Alternatively, we could specify that it doesn't throw, but instead returns a
> blob URL that can be used for Parse/Fetch on 0 bytes.

That seems wrong. Returning a blob URL that will result in a network error seems OK, though. It's not clear to me which is better, but maybe throwing early is more helpful for catching mistakes.

> BUT:
> 
> var blob = new Blob();
> var url = URL.createObjectURL(blob);
> 
> *also* returns a URL that can be used to Parse/Fetch on 0 bytes.

Indeed.

> The difference is that this by design; calling blob.close(), however, is a
> programmatic declaration that the Blob should be closed and memory should be
> culled.
Comment 15 Arun 2014-03-19 17:21:59 UTC
(In reply to Simon Pieters from comment #14)
> (In reply to Arun from comment #13)
> > Well, the reasoning is that read operations throw when you try to read a
> > closed Blob. I think this seems about right. So, minting a blob URL on a
> > closed blob should also throw, since that's a type of read but by another
> > name (Parse/Fetch -- we should fail them aggressively right out of the
> > starter gate, rather than have unpredictable Parse/Fetch behavior).
> 
> OK... I don't follow what would be unpredictable, though.


My use of "unpredictable" is misleading. It wouldn't be unpredictable, because it would follow behavior that we specify :-) Instead of "unpredictable" I meant: you'd get something like 0 bytes or a network error, without immediately knowing that the blob was closed, and thus was a no-op for all practical purposes anyway.

The two sane choices are network error vs. throwing. I favor throwing.

> 
> > Alternatively, we could specify that it doesn't throw, but instead returns a
> > blob URL that can be used for Parse/Fetch on 0 bytes.
> 
> That seems wrong. Returning a blob URL that will result in a network error
> seems OK, though. It's not clear to me which is better, but maybe throwing
> early is more helpful for catching mistakes.
> 
> > BUT:
> > 
> > var blob = new Blob();
> > var url = URL.createObjectURL(blob);
> > 
> > *also* returns a URL that can be used to Parse/Fetch on 0 bytes.
> 
> Indeed.
> 
> > The difference is that this by design; calling blob.close(), however, is a
> > programmatic declaration that the Blob should be closed and memory should be
> > culled.
Comment 16 Simon Pieters 2014-03-19 17:26:53 UTC
I think that makes things clear. No objections from me. Thanks!
Comment 17 Simon Pieters 2014-03-19 17:28:21 UTC
But please see comment 11
Comment 18 sof 2014-03-22 16:05:47 UTC
re: close(), https://www.w3.org/Bugs/Public/show_bug.cgi?id=23417 is relevant when considering how closed Blobs should be handled by platform.
Comment 19 Arun 2014-03-25 20:40:45 UTC
(In reply to sof from comment #18)
> re: close(), https://www.w3.org/Bugs/Public/show_bug.cgi?id=23417 is
> relevant when considering how closed Blobs should be handled by platform.

So, after chatting with Jonas on IRC, I think Comment 11 is right about not throwing for URL.createObjectURL and URL.createFor, since this might put the burden of try/catch on a developer just trying to coin a URL.

But this does mean that some methods will throw, and some won't.  I think FileReader.readAsxxx throwing is fine; I think close() itself throwing if closing an already closed Blob is fine. Any of those failing differently seems confusing.

Based on Comment 11, it seems your suggestion is to have the behavior be when called on a closed blob, URL.create* will return a URL which when Parse/Fetch'd will return a network error. OK.
Comment 20 Arun 2014-03-26 19:58:20 UTC
Fixed. http://dev.w3.org/2006/webapi/FileAPI/#creating-revoking

(Reopen if necessary).
Comment 21 Simon Pieters 2014-03-27 13:39:42 UTC
So you have changed your mind about createObjectURL?

(In reply to Arun from comment #19)
> So, after chatting with Jonas on IRC, I think Comment 11 is right about not
> throwing for URL.createObjectURL and URL.createFor,

Comment 11 is about close().

> since this might put the
> burden of try/catch on a developer just trying to coin a URL.

Yeah.

> But this does mean that some methods will throw, and some won't.  I think
> FileReader.readAsxxx throwing is fine; I think close() itself throwing if
> closing an already closed Blob is fine. Any of those failing differently
> seems confusing.

So the benefit you see with close() throwing is that it's consistent with readAs*? I think that's not so convincing. I think similar reasoning for createObjectURL() applies to close() -- throwing puts the burden of try/catch on a developer just trying to close a Blob. (Reopening for reconsideration/clarification about close().)

> Based on Comment 11, it seems your suggestion is to have the behavior be
> when called on a closed blob, URL.create* will return a URL which when
> Parse/Fetch'd will return a network error. OK.

s/11/14/ ?

I think that's OK, yes.
Comment 22 Anne 2014-03-27 14:34:34 UTC
So we want something that works for bug 25081 comment 6.

If a blob that is closed simply loses the reference to its underlying source and returns an empty byte sequence, that does not seem great for FormData as it would be somewhat racy. However, returning the empty byte sequence if its closed seems good.

Maybe blobs should have a keepalive list of objects. Then at the point where a FormData object (FD) is serialized, FD adds itself to the keepalive list. Then when FD starts reading from the blob it passes itself as a parameter to read and is allowed to bypass the closed check and reach the source because it is on the keepalive list. Then once the read completes FD removes itself from the list and if the blob is closed and has an empty keepalive list it can GC.

Does that model make sense?
Comment 23 Glenn Maynard 2014-03-27 14:48:17 UTC
(In reply to Arun from comment #19)
> But this does mean that some methods will throw, and some won't.  I think
> FileReader.readAsxxx throwing is fine; I think close() itself throwing if
> closing an already closed Blob is fine. Any of those failing differently
> seems confusing.

Reading from a Blob with FileReader shouldn't throw.  It should just fail with onerror.  Developers shouldn't have to write two different error handlers to use FileReader, using two completely different mechanisms (exceptions and error events).

I agree that close() shouldn't throw, either.  The blob is in the state the developer wanted--closed--so it'll just make everyone wrap blob.close() in an empty try/catch.
Comment 24 Arun 2014-04-02 21:01:28 UTC
(In reply to Anne from comment #22)
> So we want something that works for bug 25081 comment 6.
> 
> If a blob that is closed simply loses the reference to its underlying source
> and returns an empty byte sequence, that does not seem great for FormData as
> it would be somewhat racy. However, returning the empty byte sequence if its
> closed seems good.
> 
> Maybe blobs should have a keepalive list of objects. Then at the point where
> a FormData object (FD) is serialized, FD adds itself to the keepalive list.
> Then when FD starts reading from the blob it passes itself as a parameter to
> read and is allowed to bypass the closed check and reach the source because
> it is on the keepalive list. Then once the read completes FD removes itself
> from the list and if the blob is closed and has an empty keepalive list it
> can GC.
> 
> Does that model make sense?


After today's discussion in IRC, this model makes sense. It would be a keepalive list of objects using a given Blob. FileReader should add itself to the keepalive list of objects using a given Blob, too, and then remove itself.
Comment 25 Arun 2014-04-02 21:05:26 UTC
(In reply to Glenn Maynard from comment #23)
> (In reply to Arun from comment #19)
> > But this does mean that some methods will throw, and some won't.  I think
> > FileReader.readAsxxx throwing is fine; I think close() itself throwing if
> > closing an already closed Blob is fine. Any of those failing differently
> > seems confusing.
> 
> Reading from a Blob with FileReader shouldn't throw.  It should just fail
> with onerror.  Developers shouldn't have to write two different error
> handlers to use FileReader, using two completely different mechanisms
> (exceptions and error events).


I agree with this. In fact, FileReader should *only throw* when it is invalidly invoked per spec.; that is, if it is in the LOADING state, preventing multiple invocation of an "in-use" FileReader. But it shouldn't throw when reading a Blob; it shouldn't throw on a CLOSED blob.


> 
> I agree that close() shouldn't throw, either.  The blob is in the state the
> developer wanted--closed--so it'll just make everyone wrap blob.close() in
> an empty try/catch.

This is a bit less obvious to me, but this bug is about URL.create* on a CLOSED blob, which shouldn't throw.
Comment 26 Glenn Maynard 2014-04-02 21:12:25 UTC
(In reply to Arun from comment #25)
> (In reply to Glenn Maynard from comment #23)
> > (In reply to Arun from comment #19)
> > > But this does mean that some methods will throw, and some won't.  I think
> > > FileReader.readAsxxx throwing is fine; I think close() itself throwing if
> > > closing an already closed Blob is fine. Any of those failing differently
> > > seems confusing.

> > I agree that close() shouldn't throw, either.  The blob is in the state the
> > developer wanted--closed--so it'll just make everyone wrap blob.close() in
> > an empty try/catch.
> 
> This is a bit less obvious to me, but this bug is about URL.create* on a
> CLOSED blob, which shouldn't throw.

It's relevant because of the "these two things being different is confusing" argument.
Comment 27 Arun 2014-04-02 21:15:55 UTC
(In reply to Simon Pieters from comment #21)
> So you have changed your mind about createObjectURL?


Yes; I'm sorry I've muddled comments, and inflated this bug about other things.

THIS bug is fixed:

http://dev.w3.org/2006/webapi/FileAPI/#creating-revoking

 
> (In reply to Arun from comment #19)
> > So, after chatting with Jonas on IRC, I think Comment 11 is right about not
> > throwing for URL.createObjectURL and URL.createFor,
> 
> Comment 11 is about close().

You're right. This should be a new bug, just to keep things clear: Bug 25240

 
> So the benefit you see with close() throwing is that it's consistent with
> readAs*? I think that's not so convincing. I think similar reasoning for
> createObjectURL() applies to close() -- throwing puts the burden of
> try/catch on a developer just trying to close a Blob. (Reopening for
> reconsideration/clarification about close().)


readAs* shouldn't throw on neutered Blobs. This is Bug 25241.