Skip to content

Append to file on SPIFFS does not return actual bytes writen #5987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
4 of 6 tasks
TD-er opened this issue Apr 15, 2019 · 5 comments
Closed
4 of 6 tasks

Append to file on SPIFFS does not return actual bytes writen #5987

TD-er opened this issue Apr 15, 2019 · 5 comments

Comments

@TD-er
Copy link
Contributor

TD-er commented Apr 15, 2019

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: [ESP-12]
  • Core Version: [ESP82xx Core 2.6.0-dev, NONOS SDK 2.2.2-dev(c0eb301), LWIP: 2.1.2 PUYA support]
  • Development Env: [Platformio]
  • Operating System: [Windows]

Settings in IDE

  • Module: [Generic ESP8266 Module]
  • Flash Mode: [dio]
  • Flash Size: [4MB]
  • lwip Variant: [v2 Lower Memory]
  • Reset Method: [nodemcu]
  • Flash Frequency: [40Mhz]
  • CPU Frequency: [80Mhz]
  • Upload Using: [OTA|SERIAL]
  • Upload Speed: [115200] (serial upload only)

Problem Description

I know that SPIFFS may get fragmented, therefore I have a check to see if the write to a file (append mode) on SPIFFS may return less than the number of bytes I want to write to it.
However, this check was never hit in my sketch.
I now have a check for file size before and after the write and this does show the bytes are not appended to my file when the SPIFFS is out of free sectors to write to.

        size_t filesize = fw.size();
        int bytesWriten = fw.write(&RTC_cache_data[0], RTC_cache.writePos);
        if (bytesWriten < RTC_cache.writePos || fw.size() == filesize) {
          String log = F("RTC  : error writing file. Size before: ");
          log += filesize;
          log += F(" after: ");
          log += fw.size();
          log += F(" writen: ");
          log += bytesWriten;
          addLog(LOG_LEVEL_ERROR, log);
          fw.close();
          GarbageCollection();
          writeerror = true;
          return false;
        }

N.B. fw is a File fw;

This does result in errors like:

RTC  : error writing file. Size before: 720 after: 720 written: 240

I guess the write function should return the actual number of bytes written.
But on the other hand it may be that we're looking at some cache layer here?
Also replacement of data in a file will not result in a bigger file, but it may go unnoticed if the write was unsuccessful.

TD-er added a commit to TD-er/ESPEasy that referenced this issue Apr 15, 2019
The SPIFFS may become fragmented, so it is possible a file cannot be appended to even though there is room on the file system.
So there has to be a proper check to see if writing fails and also delete oldest file(s) when needed + call to the garbage collector.
See also esp8266/Arduino#5987
@earlephilhower
Copy link
Collaborator

If this is the case, then there is an intenal SPIFFS filesystem issue and there's not much we can do about it. SPIFFS::write is a simple wrapper around the SPIFFS_write internal call and we just return the value it gives us (which should be the size of data writter).

    size_t write(const uint8_t *buf, size_t size) override
    {
        CHECKFD();

        auto result = SPIFFS_write(_fs->getFs(), _fd, (void*) buf, size);
        if (result < 0) {
            DEBUGV("SPIFFS_write rc=%d\r\n", result);
            return 0;
        }
        _written = true;
        return result;
    }

I doubt we can do much here about it.

@d-a-v
Copy link
Collaborator

d-a-v commented Apr 16, 2019

file.size() calls SPIFFS_fstat() from _getStat() and has a debug message.
Can you enable core debugging option and check for messages ?
You can also enable spiffs debugging by editing cores/esp8266/spiffs/spiffs_config.h

What happens if you close/reopen the file and check its size ?
(of course it is assumed your are not overwriting a file but appending to its end)

@TD-er
Copy link
Contributor Author

TD-er commented Apr 16, 2019

@d-a-v I will try to make a build using debugging. (would be nice if that could be done using a simple define in PlatformIO, is that possible?)
The thing is, it will not happen all the time, so it may be a bit tricky to get it reproduced.
I am quite sure I don't have time today to make a test build, since I will be away for most of the day.

As you can see in my example code, the file is not closed, nor do I call a flush to the filesystem. The filesize is just checked right after the write command. Even then it is already visible no data has been appended.
And indeed, I am appending to a file.

@TD-er
Copy link
Contributor Author

TD-er commented Apr 16, 2019

Just a quick analysis of the write code in the SPIFFS library:

We have several scenarios:

  • small write (len < (s32_t)SPIFFS_CFG_LOG_PAGE_SZ(fs))

  • big write (no cache, but existing cache may need to be flushed)

  • small write with current file descriptor in cache (just returns intended length to write) <=== the issue I'm seeing.

  • small write with current file descriptor not cached (returns result of calling spiffs_hydro_write )

@earlephilhower
Copy link
Collaborator

Closing as a won't-fix since SPIFFS is being deprecated. LittleFS has its own issues, sometimes, but will be the actively supported FS for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants