150157 - hang when storing file to HDD with not enough empty space

Status: VERIFIED FIXED

(NSPR :: NSPR, defect)

Description

[Frank Nestel]

Mozilla 1.0 (final) hangs with 100% CPU
when trying to save a file to a harddisk which has not
enough empty space.

Comment 1

[Mike Kaply [:mkaply]]

Is the TMP directory on a drive that has no room or is the download to a drive 
that has no room?

First we download to TMP and then move it to the right place.

I'm not sure what we can do about this. We don't know how big the file is when 
we download it, so we can't check to see if it will fit.

Comment 2

[Frank Nestel]

The HD without was the target drive, not the TMP drive.

And actually in some cases the browser knows the size
of the file beforehand, it is able to display a progress 
bar.

Anyway, anything can happen, the safe can be canceled, a 
warning can be displayed, but having the CPU on 100% and
the Workplace Shell not reacting, only because Mozilla 
cannot
safe a file is IMHO not an acceptable state.

Regards,
Frank

Comment 3

[Mike Kaply [:mkaply]]

I tried this and I get an error message that the file was unable to be saved. I
did not get a hang.

I tried to save a 1.2 meg file to a drive with 80k free.

What was the file you were trying to save? Was it FTP, http, or saving a web page?

Comment 4

[Frank Nestel]

Hmm, I were storing a sequence of images (jpegs) via http protocoll.
The first one got stored and truncated until the HD was on exactly 
0 bytes free. Storing the next picture hung up my workplace shell.
I had the same effect again yesterday. I can reproduce it on my
one and only OS/2 system arround (this is the problem with OS/2 systems).

Comment 5

[Mike Kaply [:mkaply]]

Was the harddrive that ran out of space the location of your OS2 directory or 
your SWAPPER.DAT?

Comment 6

[Mike Kaply [:mkaply]]

Got it. Only happens with saving images.

Comment 7

[Mike Kaply [:mkaply]]

Moving to NSPR - bug in OS/2 that can be worked around in NSPR

Comment 8

[Mike Kaply [:mkaply]]

Moving to NSPR

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

Very strange.

For some reason, DosWrite is not returning an error when the disk is full.

We can tell that the write failed, however, by comparing what we were trying to
write with what actually got written.

Comment 10

[Wan-Teh Chang]

Michael,

Do you think we should change the return value to -1
only if 'bytes' is 0 and return the partial write
byte count if 'bytes' is > 0?

Comment 11

[Mike Kaply [:mkaply]]

Attachment: Better fix per wtc

Yeah, that is probably a more correct way to do things.

I've tested both scenarios and they work fine.

I will discuss it with the OS/2 team tomorrow to find out why we aren't getting
the error code.

Thanks!

Comment 12

[Scott Garfinkle]

This is neither a Mozilla nor a generic OS/2 bug, I think. I assert (without 
having spent time on it) that it's specific to a specific filesystem. Unless I 
missed it, there is no mention here either of the type of filesys in question 
(FAT, HPFS, JFS, etc.) or of the fix level of the OS. There was, for example, a 
problem like this with JFS in the GA (i.e. first released) version of Warp 
Server for e-Business 9fixed in later releases). Please specify the IFS 
involved, as well as the OS/2 level. -Scott 

Comment 13

[Scott Garfinkle]

There is also another possibility that comes to mind. I think that, after re-
reading the test, that there may, in fact, be a Mozilla problem. If you know a 
file size and try to create a file of a given length and it won't fit on disk, 
then you definitely should get back ERROR_DISK_FULL from DosOpen. However, 
partial writes are allowed, so if Mozilla tries to write say 80kb and there's 
only 40kb left on disk, we are SUPPOSED to return with NO_ERROR and then it's 
the app (e.g. Mozilla) that has to check the number of bytes actually written 
(returned by DosWrite). That sounds like what's happening here. The shell 
probably hung because it tried to write an INI file or page out to the page 
file (swapper.dat) on the now-full drive. Bad things happen when you run out of 
disk space. Don't do it. -Scott

Comment 14

[Frank Nestel]

Is it me to answer the bug questions? Well,
* The WPS not really hangs, actually CPU is on 100% and it is impossible to work
with the system. But the full HD-partition does not contain swap, ini or
whatever system files, it is a pure data partition.
* The filesystem is a good old hpfs. Since the machine is usually working fine
it might be a fairly old fixpak (if you need to know, how do I check hpfs version?)

Comment 15

[Mike Kaply [:mkaply]]

wtc,

I think there might be a subtle NSPR bug here, but maybe not. You decide. In 
prfile:

http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/io/prfile.c#106

There is the potential for an endless loop if the write returns 0 and not -1. I 
could see a theoretical case where the write succeeded, but wrote 0 bytes. In 
this case, this loop would never terminate.

Perhaps there should be some extra checking to verify that amount is actually 
decreasing.

Comment 16

[Wan-Teh Chang]

Mike,

You are right that if the write returns 0 PR_Write will get
in an endless loop.  Looking at the code I think NSPR
implicitly assumes that the write returns either a positive
byte count or -1.

PR_Write could detect that _PR_MD_WRITE returns 0 and break
out of the while loop, but it's not clear what the error
code should be set to.  So I think the best solution is for
each platform to detect a return of 0 by the write, set
the error code, and return -1 in _PR_MD_WRITE.

Comment 17

[Mike Kaply [:mkaply]]

wtc, could r= this? This is a good one to try to get on the branch. thanks

Comment 18

[Wan-Teh Chang]

Comment on attachment 88017 [details] [diff] [review]
Better fix per wtc

>+    if (len != bytes) {
>+        rv = ERROR_DISK_FULL;
>+        _PR_MD_MAP_WRITE_ERROR(rv);
>+        if (bytes == 0) {
>+           return -1;
>+        } else {
>+           return bytes;
>+        }
>+    }
>+

This should say:

      if (bytes == 0 && len != 0) {
	  _PR_MD_MAP_WRITE_ERROR(ERROR_DISK_FULL);
	  return -1;
      }

Comment 19

[Mike Kaply [:mkaply]]

I don't agree. If len != bytes, there is a DISK_FULL_ERROR regardless of what we 
return.

So we should set the DISK_FULL_ERROR anytime len != bytes, and only return -1 
if bytes = 0.

Comment 20

[Wan-Teh Chang]

Actually, I think we should use the first patch.  If you
look at how FileWrite (in prfile.c) calls _PR_MD_WRITE,
you will see that both patches would produce the same
result (PR_Write returns -1 with DISK_FULL_ERROR).

Correct?

Comment 21

[Mike Kaply [:mkaply]]

All right then. Let's get this thing reviewed and in.

Thanks

Comment 22

[Wan-Teh Chang]

Comment on attachment 88012 [details] [diff] [review]
Fix for problem

r=wtc.

Comment 23

[Wan-Teh Chang]

I checked in the first patch on the trunk and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (used by the
Mozilla trunk) of NSPR.

If you would like to check it into the
Mozilla 1.0 branch, please get the
mozilla1.0.1+ approval from the drivers.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 88012 [details] [diff] [review]
Fix for problem

Please land this on the 1.0.1 branch.  Once there, replace the
"mozilla1.0.1+" keyword with the "fixed1.0.1" keyword.

Comment 25

[Mike Kaply [:mkaply]]

fix in on branch

Comment 26

[Mike Kaply [:mkaply]]

verified - this is fixed