Disk full error not being raised appropriately when file is re-opened

[victorallume]

Describe the bug
There seems to be a bug when a file is re-opened for writing, that disk usage is not calculated correctly, and hence does not throw disk full exceptions at the right place. In the code below, in the first test, the assertion fails. The second test works fine. The third function fails the assertion. I discovered this error in code which re-opens files regularly, and uses random-access read/write mode ('r+'), and seeking within files.

How To Reproduce

class ExampleTestCase(TestCase):
    def setUp(self):
        self.setUpPyfakefs()
        self.fs.set_disk_usage(100)

    def test_disk_full_reopened_160(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'w') as f:
            with self.assertRaises(OSError):
                f.write('b' * 160)
                f.flush()

    def test_disk_full_reopened_161(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'w') as f:
            with self.assertRaises(OSError):
                f.write('b' * 161)
                f.flush()

    def test_disk_full_reopened_rplus_seek(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'r+') as f:
            with self.assertRaises(OSError):
                f.seek(50)
                f.write('b' * 60)
                f.flush()

Your environment
Please run the following and paste the output.

Linux-5.15.11-76051511-generic-x86_64-with-glibc2.34
Python 3.9.7 (default, Sep 10 2021, 14:59:43) 
[GCC 11.2.0]
pyfakefs 4.5.4

[mrbean-bremen]

Thanks for the report! This certainly looks like a bug in pyfakefs, seems that these cases are not handled properly. I will have a closer look tonight.

[victorallume]

An even simpler test which I think might be at the heart of the problem:

    def test_disk_usage(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
            f.flush()
        self.assertEqual(self.fs.get_disk_usage()[1], 60)

[mrbean-bremen]

@victorallume - this was a clear bug, probably a leftover from some code change - thanks again for the report. Please check if it works for you in master. If this and the other included bugfix work, I will probably make a bugfix release.

[victorallume]

Thanks; a quick check of the tests and it looks like master works fine.

A side note though; the following test works exactly as expected now:

    def test_disk_full_append_160(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'a') as f:
            with self.assertRaises(OSError):
                f.write('b' * 41)
                f.flush()

but the test below raises an OSError at runtime on the last line f.flush() which should be caught by the context manager, which works fine on the above code, and the test fails. I've also tried a non-context-manager assertRaises, but this has the same result. Any ideas? Could this be related to pyfakefs?

    def test_disk_full_reopened_rplus_seek(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'r+') as f:
            with self.assertRaises(OSError):
                f.seek(50)
                f.write('b' * 60)
                f.flush()

Both tests are in the test class listed below.

class ExampleTestCase(TestCase):
    def setUp(self):
        self.setUpPyfakefs()
        self.fs.set_disk_usage(100)

[victorallume]

Even the following code still fails with an OSError, even though the code in the except clause runs:

    def test_disk_full_reopened_rplus_seek(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        with open('bar.txt', 'r+') as f:
            # with self.assertRaises(OSError) as e:
            try:
                f.seek(50)
                f.write('b' * 60)
                f.flush()
            except OSError as e:
                self.assertIsInstance(e, OSError)

[mrbean-bremen]

Yes, the exception is caught for flush, but the open context manager calls close on exit, which also does a flush and will raise the exception again. You can can check this by removing the explicit flush and put the exception handler around the open block.

[mrbean-bremen]

The problem is that the write call worked, and the data is already in the buffer, so flush will be called on closing the file and it will raise again. I think that this is the expected behavior.

[victorallume]

Thanks; that makes sense (the below code works fine). But why does the first example (test_disk_full_append_160) not throw the error again when the context manager closes the file?

    def test_disk_full_reopened_rplus_seek(self):
        with open('bar.txt', 'w') as f:
            f.write('a' * 60)
        with open('bar.txt') as f:
            self.assertEqual(f.read(), 'a' * 60)
        f = open('bar.txt', 'r+')
        with self.assertRaises(OSError):
            f.seek(50)
            f.write('b' * 60)
            f.flush()
            f.close()

[mrbean-bremen]

You are right, this behaves differently - I missed this. Apparently, the flush position is reset before the exception is raised, so in the case of append the second flush will not try to append anything, which is wrong.
I will have another look later today, also will probably try to test the behavior in the real OS before fixing this to be sure that the behavior will match.

I don't think that it will matter much in practice though, as the first flush (either explicit, or. more commonly, via close) will raise the exception, and the problem only occurs if you do an explicit flush and catch the exception in that call.

[mrbean-bremen]

Should be corrected now. I verified that the real filesystem is behaving the same way. Note though that you cannot rely on the contents of the file written which caused the out of space exception - this depends on the buffering and will differ between real fs and pyfakefs.

Edit: that last sentence wasn't actually true, I did a mistake while testing this. It should behave as in the real fs.