Closed Bug 340956 Opened 15 years ago Closed 15 years ago

PR_Write in append mode fails for big files (4GB+/32bit border)


(NSPR :: NSPR, defect)

Windows XP
Not set


(Not tracked)



(Reporter: nmaier, Assigned: wtc)



(Keywords: fixed1.8.0.5, fixed1.8.1)


(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv: Gecko/20060508 Firefox/
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv: Gecko/20060508 Firefox/

PR_Write "emulates" the O_APPEND flag by seeking to EOF upon each write when in append mode.

This is fine, but PR_Seek instead of PR_Seek64 is used so that this fails one we pass the 4GB "border".

I think this at least affects TB (no folders >4GB) as well as the downTHEMAll extension (found this hacking the latter).

Reproducible: Always

Steps to Reproduce:
1. Open an nsIFileOutputStream and write more than 4GB data. (Not at once)
2. See it throw once 4GB mark is reached.
Nils: thank you for the bug report.  I believe the fix
is to change PR_Seek to PR_Seek64 as you pointed out.
Since you have the setup to test the fix, could you
submit a patch and test it?

Darin, it may be a good idea to review all the uses
of PR_Seek and see if any of them needs to be changed
to PR_Seek64:
Ever confirmed: true
Attached patch v1 (obsolete) — Splinter Review
tested using a trunk build from a freshly pulled tree.
works with:
XP SP2 (Pro)
W2k SP4

PS: I lack everything, knowledge of the review process and obviously checkin privileges.
This causes bug 331647.
With this patch, I could download Windows Vista beta 2 x64 successfully :)
Blocks: 331647
Comment on attachment 225092 [details] [diff] [review]

> -        rv = PR_Seek(fd, 0, PR_SEEK_END );
> +        rv = PR_Seek64(fd, 0, PR_SEEK_END );
|rv| has only 32 bit width. What happens if PR_Seek64 returns 0xffffffff?
Attachment #225092 - Flags: review-
Attached patch using PROffset64 (obsolete) — Splinter Review
Attachment #225092 - Attachment is obsolete: true
Attachment #225105 - Flags: review?(wtchang)
Attachment #225105 - Flags: approval-branch-1.8.1?(wtchang)
err. turns out my first submitted piece of code is actually a piece of...
Was too easy to get it right.

Thanks for the correction.

Looking even more at this reveals this:
PROffset64 -> PRInt64
PRInt64 might be a struct if there is no __int64 and long long.
Remarks say LL_ macros should be used to ensure portable code.

I leave it to you experienced guys to figure this out. ;)
I heard LL_ macros are obsolete from Darin on bug 333703.
Does NSPR still support no __int64 platforms unlike Mozilla product?
Per bug 308587, NSPR also doesn't support platforms without native 64-bit integer type anymore.
Comment on attachment 225105 [details] [diff] [review]
using PROffset64

Good catch for the PR_Seek64 return type issue
and good code cleanup.

Let's use a different variable name such as 'offset'
or 'off' for the return value of the PR_Seek64 call,
or just say:

+        if (PR_Seek64(fd, 0, PR_SEEK_END) == -1) {
+            return -1;

On the LL_ macros: new code doesn't need to use the
LL_ macros because all the compilers that we use now
support a 64-bit integer type, which may be 'long long',
'long', or '__int64'.
Attachment #225105 - Flags: review?(wtchang)
Attachment #225105 - Flags: review+
Attachment #225105 - Flags: approval-branch-1.8.1?(wtchang)
Attachment #225105 - Flags: approval-branch-1.8.1+
Thanks for your review. Carrying over r+branch181.
Could you land the patch, please?
Attachment #225105 - Attachment is obsolete: true
Attachment #225122 - Flags: review+
Attachment #225122 - Flags: approval-branch-1.8.1+
I think we should consider this for 1.8.0.x too.
Flags: blocking1.8.0.6?
Flags: blocking1.8.0.5?
Nils, I'm curious how you tracked down this bug.  This
bug takes a 4GB download to reproduce.  It's not obvious
how you catch it in action at the right moment.

We need to test this fix on Windows and OS/2.  This fix
does not affect BeOS and Unix (including Linux and Mac
OS X).

I checked in the bug on the NSPR trunk (NSPR 4.7 Beta),
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9 alpha),
NSPR_4_6_BRANCH (NSPR 4.6.3 Beta), and MOZILLA_1_8_BRANCH
(Mozilla 1.8.1 beta1).

Checking in prfile.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v  <--  prfile.c
new revision: 3.45; previous revision: 3.44

Checking in prfile.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v  <--  prfile.c
new revision:; previous revision:

Checking in prfile.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v  <--  prfile.c
new revision:; previous revision:

Checking in prfile.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v  <--  prfile.c
new revision:; previous revision:
Closed: 15 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Target Milestone: --- → 4.6.3
Download of WinVista Beta2 ISO was reported to always fail with downTHEMall.
First I replicated that problem.
Downloading a 4.3GB file from my local apache.
Using sysinternal's filemon. Which showed a failed seek.
There is no seek() in dTa, Windows won't seek, so it must be Firefox.

Then I simply went to lxr and traced the callstack :p
nsFileOutputStream uses PR_Write.
PR_Write is fd->methods->write.
which is a member of PRIOMethods declared in prio.h and referenced by prio.c
which then gives FileWrite, i.e. the buggy function.
Saw the PR_Seek (and the FileSeek and FileSeek64 near FileWrite).

Was hidden damn well. Took some time to find it.
Flags: blocking1.8.0.6?
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5-
Comment on attachment 225122 [details] [diff] [review]
resolved review comment

Not blocking, but approved for 1.8.0 branch, a=dveditz for drivers
Attachment #225122 - Flags: approval1.8.0.5+
I checked in the patch on the MOZILLA_1_8_0_BRANCH

Checking in prfile.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prfile.c,v  <--  prfile.c
new revision:; previous revision:

Nils, it seems that the "breakthrough" in your debugging
was that sysinternal's filemon showed a failed seek.
That pointed you in the right direction.
Keywords: fixed1.8.0.5
Duplicate of this bug: 299598
You need to log in before you can comment on or make changes to this bug.