459384 - History traversal back to POST result page locks file that was submitted

Status: NEW

(Core :: Networking, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

See bug 183689 comment 7.

The basic problem is that we seek to the beginning of the POST data stream, as described in bug 183689 comment 37.  Then HTTP doesn't actually read the data, and the file stays open.

I guess we could just read all the data in this case, but maybe there's a better approach.  I'll poke around a bit.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So I don't see anything in the stream APIs that would let us do this... we want to close the file stream but NOT the multiplex and buffered streams wrapping it.  Any ideas?  I suppose we could add an interface that allows closing underlying file streams and have multiplex and buffered streams implement it.

I really don't want to be reading all the POST data just to get the file we don't actually need properly closed.  :(

Another option, I guess, is to move the seek from docshell into necko so we don't reopen the file until we really need it.

Comment 3

[David E. Ross]

Isn't this backwards?  Shouldn't this bug be closed as a duplicate of the earlier bug 183689?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Age of the bugs isn't the only consideration to make when choosing to mark a duplicate. The value of the comments, existence of a patch or patches, and presence of the right people in the CC list are much more important.

Comment 5

[Boris Zbarsky [:bzbarsky]]

And clarity of the bug, which was the main reason for the dup direction here.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This is the solution we discussed on IRC.  I can't seem to reproduce this, so I can't really tell whether or not it works :-|

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

hg qref

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 485490 [details] [diff] [review]
Patch

Sorry for the lag here...

This is wrong on at least three counts:

1)  In the mUploadStreamHasHeaders == true case, this is setting the wrong
    content-length.
2)  Doing the rewind/etc in AsyncOpen could leave us in this bug's situation,
    still, if we get a cache hit and just read from cache.  We need to push that
    off to the point where we'll actually use mUploadStream.
3)  For the file input stream, using the cached last return of Available()
    doesn't make that much sense to me...  Wouldn't we want to return a cached
    file size instead?

Comment 12

[Jan S.]

Comment on Bug 459384 and Bug 183689

The issue still exists in firefox 51.0.1. File handle stays open until browser is closed. It only happens if firefox opens more than one process - e.g. AddOns like Ghostery are activated.

Process monitor shows that for one process the file handle gets released but the other firefox process still holds it.


Reproduce:
- install Ghostery AddOn
- check if more than 1 process of firefox is started
- upload file at: http://validator.w3.org/#validate_by_upload
- try to remove file with DEL command from cmd

Comment 13

[Boris Zbarsky [:bzbarsky]]

Please file a separate bug on the Ghostery issue.  It's not related to this bug.

Comment 14

[Jan S.]

Ghostery was just an example how to force firefox to open a second process and use its [e10s]. I filed a separate bug report here: https://bugzilla.mozilla.org/show_bug.cgi?id=1343509

Comment 15

[Boris Zbarsky [:bzbarsky]]

I'm clearly not working on this....  Necko's API just needs to get fixed.  :(  Needs a new necko owner.

Comment 16

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 17

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.