Closed Bug 921812 Opened 9 years ago Closed 8 years ago

[Buri][fugu][MMS]The message won't display after send successfully if the original file is deleted

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect, P2)

defect

Tracking

(blocking-b2g:-)

RESOLVED WORKSFORME
blocking-b2g -

People

(Reporter: sync-1, Unassigned)

Details

Attachments

(2 files)

24.08 KB, image/x-png
Details
31.60 KB, application/octet-stream
Details
Mozilla build ID:20130916041201
 Created an attachment (id=527264)
 pic
 
 DEFECT DESCRIPTION:
  [MMS]The message won't display after send successfully.
 
  REPRODUCING PROCEDURES:
  1.Message->Create new MMS->Add recipient->Add a picture from gallery->Press home key back to idle
  2.Enter gallery to delete the picture->Enter message->Send the MMS
  3.The MMS can't display when sending--K.O1
  4.The picture can't display when MT receive the MMS--K.O2
 
  EXPECTED BEHAVIOUR:
  K.O1--MMS should display sending in message dialog.
  K.O2--The picture should disply normally.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
  Medium
  REPRODUCING RATE:
  5/5
  For FT PR, Please list reference mobile's behavior:
Clone from brother
Attached image pic
Clone from brother
Attached file PR530109_Log
Hey Ben,

I don't know if you could help here, or show us at least where to try.

The issue (as I understand it) is that we get a blob from the Gallery, but then the underlying file is deleted, and (maybe) the blob is freed?

I think the same blob is being stored as indexed DB in the SMS Gecko API when we're sending, but if it's not "existing" anymore, maybe the API is failing (or is it indexedDB?).

Thanks for any information you could give :)
Flags: needinfo?(bent.mozilla)
Hm, if we've stored the file in indexedDB then even if the original file is deleted we should be able to retrieve it. (Storing files into indexedDB copies them).

If the file is deleted before we store it into indexedDB but the blob is still alive then I think we should be ok (the blob holds a handle to the file). If the blob is freed then I think we're out of luck.

Does everything work if the file is not deleted?
Flags: needinfo?(bent.mozilla)
(In reply to ben turner [:bent] (needinfo? encouraged) from comment #6)
> Hm, if we've stored the file in indexedDB then even if the original file is
> deleted we should be able to retrieve it. (Storing files into indexedDB
> copies them).

It's stored in indexedDB only after we send, so in this case we haven't stored anything yet.

> 
> If the file is deleted before we store it into indexedDB but the blob is
> still alive then I think we should be ok (the blob holds a handle to the
> file). If the blob is freed then I think we're out of luck.

In our case, I think we always keep a reference to a valid blob object. I'm wondering whether the blob data is freed because the blob object is freed in the origin app (Gallery) even if we keep a blob reference in the SMS app (which would be a bug). Would that be possible?

> 
> Does everything work if the file is not deleted?

Yep everything works perfectly in that case!
Flags: needinfo?(bent.mozilla)
(In reply to Julien Wajsberg [:julienw] from comment #7)
> In our case, I think we always keep a reference to a valid blob object. I'm
> wondering whether the blob data is freed because the blob object is freed in
> the origin app (Gallery) even if we keep a blob reference in the SMS app
> (which would be a bug). Would that be possible?

No, all blobs are reference counted in the parent process so this should not be possible (of course, there could be bugs...). I think the problem here is that we don't currently have a way to prevent the file from being deleted.

Why is this file being deleted, by the way?
Flags: needinfo?(bent.mozilla)
Summary: [Buri][MMS]The message won't display after send successfully. → [Buri][fugu][MMS]The message won't display after send successfully.
Should we have warning msg to users as comment#7?
Flags: needinfo?(swilkes)
Flagging Ayman as this affects messaging.
Flags: needinfo?(swilkes) → needinfo?(aymanmaat)
Hi Steve 

I would like to explore possibilities for preventing an attached file from being deleted when the original file is deleted before we start handing this through the implementation of warning/error messages.

ni? to Julien. What are our options here, and what is the dev overhead?
Flags: needinfo?(aymanmaat) → needinfo?(felash)
(In reply to ben turner [:bent] (use the needinfo? flag!) (gone until dec. 6!) from comment #8)
> (In reply to Julien Wajsberg [:julienw] from comment #7)
> > In our case, I think we always keep a reference to a valid blob object. I'm
> > wondering whether the blob data is freed because the blob object is freed in
> > the origin app (Gallery) even if we keep a blob reference in the SMS app
> > (which would be a bug). Would that be possible?
> 
> No, all blobs are reference counted in the parent process so this should not
> be possible (of course, there could be bugs...). I think the problem here is
> that we don't currently have a way to prevent the file from being deleted.

Just so that I understand correctly:

* with DeviceStorage we get a File object (that is usable as a Blob)
* this File object is directly sent as a result to the activity, get it from the SMS side
* the file is deleted
=> the File object is still valid but it has no valid content

Is it how it works?

Would keeping an open handle on the file while the blob is valid be a possible solution? I think that on Linux at least the actual file is not deleted until all file handles are closed.

> Why is this file being deleted, by the way?

Because this is an actual action from the user (this is in the STR).


Ayman, Steven, Stephany, I think we have an actual bug here, that we should fix without UX changes.
Flags: needinfo?(felash) → needinfo?(bent.mozilla)
blocking-b2g: --- → fugu+
Steven, why is this a fugu-specific bug?

You can't nominate individual bugs like this, especially ones that are really edge-casy like this one. You can't add more deadlines and more work using this fugu flag.

IMO the fugu flag is adequate for fugu-specific issue only (eg: DSDS), and not for more general bugs like this one.

Please remove it (won't do it myself as this would be unpolite).
Flags: needinfo?(styang)
Agree it's edge-case, after discussed with our partner, it will not block fugu.
blocking-b2g: fugu+ → ---
Flags: needinfo?(styang)
(In reply to Julien Wajsberg [:julienw] from comment #12)
> Is it how it works?

Yep, I believe that is correct.

> Would keeping an open handle on the file while the blob is valid be a
> possible solution? I think that on Linux at least the actual file is not
> deleted until all file handles are closed.

I think there is/was a bug about this a long time ago (the same potential problem exists on desktop firefox). Maybe khuey knows where to look?

> Because this is an actual action from the user (this is in the STR).

Ah, I see... Hm. I'm tempted to say that we don't care about this problem. Sicking?
Flags: needinfo?(khuey)
Flags: needinfo?(jonas)
Flags: needinfo?(bent.mozilla)
I have mixed feelings: on one hand I agree with you that this specific problem is really an edge case. On other hand I feel like we could have (or already have) other similar issues in the future if we don't fix the root cause.

The main issue I see is: I have a handle on this data, why does this data disappear? This could happen for other causes as well.
(In reply to Julien Wajsberg [:julienw] from comment #16)
> The main issue I see is: I have a handle on this data, why does this data
> disappear? This could happen for other causes as well.

The data disappears if the underlying file is modified or deleted. (I don't actually think we check for modifications currently, but we should).

The thing that I want to avoid is creating a copy of the data anytime a Blob object is created, just-in-case someone goes to modify it later.

*If* we could implement Blob such that any time a Blob object is created we always open a linux OS-level file descriptor to the file, then we would be in a somewhat better state. If the file got deleted we'd still be able to read from it. However if the file were in-place modified, we'd still be out of luck.

Additionally, it's not clear that opening a OS-level file descriptor would work. At best the app could run out of file descriptors. At worst, we might run out of file descriptors in the parent process which would be terrible. This is made worse by the fact that Blobs are GCed, which means that whether you run out of file descriptors or not depends on the GC implementation. And with generational GC objects can stay around for a long time past the last reference was released.

So yeah, I don't think there's really much we can do here that would be useful. Due to in-place modifications, there's nothing we can do to guarantee that a Blob object reference guarantees access to the data. And anything we try to protect against deleted files means running the risk of running out of OS-level file descriptors.
Flags: needinfo?(jonas)
So the only way to protect against this would be to do a Blob copy of the data. I don't really want to do this on a phone, unless big blobs are being written on the "disk".

(In reply to Jonas Sicking (:sicking) needinfo? or feedback? encouraged. from comment #17)
> (In reply to Julien Wajsberg [:julienw] from comment #16)
> > The main issue I see is: I have a handle on this data, why does this data
> > disappear? This could happen for other causes as well.
> 
> The data disappears if the underlying file is modified or deleted.

Yes of course, the "I" in my sentence was a indefinite "I" ;)

> (I don't
> actually think we check for modifications currently, but we should).
> 

Do you think it's doable to have events for state changes, like "onInvalid" or something like this? This would at least make it possible to react at the application level.

Oh, BTW, MDN says: "A Blob object represents a file-like object of immutable, raw data". So is "immutable" wrong here? Should we change the documentation?
> Do you think it's doable to have events for state changes, like "onInvalid"
> or something like this? This would at least make it possible to react at the
> application level.

We can probably do that. Though it'll be non-trivial to implement, so I wouldn't expect us to implement it very soon. And keep in mind that the data can always get invalidated between the time when you request to start reading and the reading actually starts. So the event won't be 100% reliable.

> Oh, BTW, MDN says: "A Blob object represents a file-like object of
> immutable, raw data". So is "immutable" wrong here? Should we change the
> documentation?

The documentation is correct. The data never changes. But the Blob can be invalidated. The spec even has a .close() function, though we don't seem to implement that.

Also, all APIs come with limitations. While "new Array" is supposed to always create a new Array, it can also throw an exception if you're out of memory. And reading a Blob can also fail if the disk got corrupted.
Summary: [Buri][fugu][MMS]The message won't display after send successfully. → [Buri][fugu][MMS]The message won't display after send successfully if the original file is deleted
blocking-b2g: --- → 1.3?
Not a regression, so this is not a blocker.
blocking-b2g: 1.3? → -
Blobs have changed enough in the last year that I don't know that the question posed even makes sense anymore.
Flags: needinfo?(khuey)
I'm quite sure this is something that's been fixed by at least bug 1079546 (and other bugs before this).
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1079546
Mmm no sorry, I badly read the bug.

Let's test it again.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Actually the STR works for me in a recent build. Please file a separate bug if the issue comes again.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.