Closed Bug 121314 Opened 23 years ago Closed 22 years ago

ftp upload waits until mozilla exit

Categories

(Core Graveyard :: Networking: FTP, defect)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.9

People

(Reporter: Brade, Assigned: Brade)

References

Details

Attachments

(5 files, 3 obsolete files)

For some reason, uploading a file via ftp waits until mozilla exits to actually
write the data.  The file does seem to get zeroed immediately but the data can't
be retrieved on another computer until the uploading mozilla app has exited. 
Changing the pref for network.ftp.idleConnectionTimeout to 10 didn't help.

At this time, I don't have a patch which can be applied to mozilla to recreate
this but the majority of the patch is in bug 120659.

I will attach a log in a moment.
Attached file log file
note: this is what I did to generate this log
 * launch mozilla
 * open web location for new Composer window with ftp url
 * type some text to enable publish menu item
 * choose File > Publish
 * in browser, load ftp url for parent directory
 * quit
According to the log you loaded ftp://bubblegum.mcom.com/cmanske/sss.html, then
stored to it. Is that correct?

I don't see "load ftp url for parent directory" in there, although that is
cached, so it may not me. Theres a macsockopt assertion - is that normal? Can I
get a packet log?
update:  ftp upload worked fine (immediately) on a Solaris box within my local
network.  The ftp server I was testing with earlier was an IIS server.

At this time, I'm not able to get a packet log because I am using SERA to
connect behind the firewall.

regarding the log and what you see there:
  Composer first loads the ftp url so you can edit it
  Composer then tries to publish it (STOR)
so yes, that is correct.

The assertion is something I have been seeing for quite some time; I see it when
I view ftp urls in the browser and load them in Composer.  I don't think I see
this assertion when I push up the file.
Just to summarise what I said to brade on IRC, its possible that this is a
server problem, where mozilla does something different to ns4.

benc is on sabbatical, I believe - Tom, can you a log of ns4 uploading to
bubblegum via ftp, and attach it?
Keywords: qawanted
Any update on this? Tom?
This attachment is what 4.x does; it correctly publishes the file to bubblegum
This attachment is what 6.x did (as of last week).
You'll notice that the STOR command is issued after the data is sent.
I re-logged several times and it was always the same.  
I put breakpoints in *_Stor and the data was sent before that breakpoint was
hit.
Is there an ms ftp server I can get access to?
Target Milestone: --- → mozilla1.0
only nsbeta1+ bugs can have milestones, resetting to ---
Target Milestone: mozilla1.0 → ---
We have to have this for publishing--nsbeta1+/1.0
Charley hasn't seen problems on Windows so maybe it's a mac-specific problem?
Keywords: nsbeta1+
Target Milestone: --- → mozilla1.0
I see it on Windows, the STOR holds the ftp open.
Attached patch possible patch (obsolete) — Splinter Review
OK, try this. this should fix the problem of sending data before we get the
STOR response. Can you please let me know if this fixes that problem, even if
it doesn't fix the actual bug?
brade: Did the patch help?
I tried publishing with that patch but it doesn't publish at all (even after
quitting mozilla).  Unfortunately, I can't easily get an ftp log right now to
see where the failure is occurring.
I tried this patch as well (Windows 2K), and unfortunately it breaks uploading 
totally for me too :( 
As brade said, FTP upload has been working for me, sort of:
The file does upload completely, but I do not see the final 
nsIWebProgressListener::onStateChange notification with 
(STATE_STOP | STATE_IS_NETWORK), which I assume is how we are supposed to tell if
upload is completed. 
I'll attached logs soon.
Blocks: 88208
This is with trunk tree on 2/20/02, no patch applied
Thats not this bug... Let me look into why the patch breaks stuff, though. It
definately worked for me.
Actually, this is the same bug. We never close the data pipe after we've
finished with it, so teh server thinks that we're still going, and never sends
the completion response, so we think its still going, and never send the stop
notification.
Status: NEW → ASSIGNED
OK, the problem is that nsStorageStream::ReadSegments doesn't check for eof. 

<darin> i think some code in nsStorageStream needs to change to recognize
nsIOutputStream::Close()
<darin> and translate that to cause nsiInputStream::Read to return NS_OK w/ zero
bytes read when at the end of the buffer
<darin> the calling code should close/release its nsIOutpuStream that it got
from nsIStorageStream::NewOutpuTStream
<darin> *before* it gives out an upload stream
<darin> i think this is brade's bug

Because of this, we don't get teh onstop, so we don't tell anyone about it. That
is cmanske's problem.

I'm not sure if this is sufficient to fix the original issue - I may need to
cancel the request (and set the transport to not be reused?) from onstop. I
can't test that until I actually get the onstop, though.

->brade, -qawanted
Assignee: bbaetz → brade
Status: ASSIGNED → NEW
Keywords: qawanted
brade: this definitely has two parts to it.

1) nsStorageStream is designed to allow one guy to write while to another guy is
reading.  if the reader catches up to the writer, then the reader sees a
NS_BASE_STREAM_WOULD_BLOCK error.  however, if there is no writer, then when the
reader reaches the end of the stream, we can return NS_OK to indicate EOF.

2) in order for this to work correctly, the owner of the storage stream's output
stream has to release it (or close it).

let me know if you need any help w/ the storage stream changes.
*** Bug 126261 has been marked as a duplicate of this bug. ***
I will work on these stream issues but what about the original reason I filed
this bug:
  STOR command is sent *AFTER* data is sent?
Status: NEW → ASSIGNED
Attachment #68230 - Attachment is obsolete: true
Attachment #70572 - Attachment is obsolete: true
I don't think that that is the issue - the connection is already open, so tcp
will handle buffering until the server reads from it. I think that your fix will
fix it. I'll test it in a bit. 
brade: your changes look good to me, except shouldn't StartUpload take a
nsIStorageStream parameter instead of a nsIOutputStream?  it seems to QI to
nsIStorageStream immediately, and it also seems odd to close a stream and then
pass the same stream to StartUpload.  how about doing the QI outside of the call
to StartUpload?
Attachment #70769 - Attachment is obsolete: true
Comment on attachment 70942 [details] [diff] [review]
patch with changes suggested by Darin

r=adamlock
Attachment #70942 - Flags: review+
Tested by me with Composer publishing, and we now get the correct "end of file
transfer" messages. Hurray!
Comment on attachment 70942 [details] [diff] [review]
patch with changes suggested by Darin

sr=darin (nice job fixing this one!)
Attachment #70942 - Flags: superreview+
OS: Mac System 9.x → All
Hardware: Macintosh → All
Target Milestone: mozilla1.0 → mozilla0.9.9
Comment on attachment 70942 [details] [diff] [review]
patch with changes suggested by Darin

a=shaver for 0.9.9, nice work all around!
Attachment #70942 - Flags: approval+
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Since this is a transport problem I should be able to see, I'm keeping this for
verification.
Keywords: verifyme
VERIFIED:
I've been running some functional tests (which involve using a lot of netstat
output) and publishing while using composer at the same time, and I don't see this.

I'll talk to sujay about adding some checks in his composer functional tests
because I don't test composer on a regular basis.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: