Skip to content
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

Fix Android bug on FAT32/exFAT SD card time resolution and mtime caching #360

Merged
merged 19 commits into from
Oct 6, 2024

Conversation

lmagyar
Copy link
Contributor

@lmagyar lmagyar commented Jun 10, 2024

As I wrote in #351 (comment), Android lies about the lastModifiedTime. Even the specs says, that if the time resolution is lower than what we ask, the next getLastModifiedTime will give back the "truncated" value, this is simply not true (tested on real phone, Android 9.0 and emulator). It gives back the truncated values only when he wants, usually hours later. This is an unpredictable nightmare.

This fix:

  • adds a new method to the StorageManagerUtil class that detects SAF filesystems from /proc/mounts, it detects:
    • FAT16, FAT32 -> 2000ms
    • exFAT -> 10ms 2000ms
    • default 1ms
  • moves the fileSystemView property from SafSshFile up to SafFile, so all SAF file can call the new getTimeResolution() from SafFileSystemView

Update:

  • also implemented for roSAF and poFS
  • poFS alters mtime only for files on the SD card's, ie. under path eg. /storage/XXXX-XXXX

Maybe the default 1ms is overkill, in theory FTP has sub-second resolution, though we can modify StorageManagerUtil to return only 1000 vs 2000 ms, in this case no overload needed in SafSshFileSystemView to limit sub second resolution to the 1s resolution of sftp messages.

@lmagyar
Copy link
Contributor Author

lmagyar commented Jun 10, 2024

Not the latest logs, but some proof it works.
Emulator:

getFilesystemTimeResolutionForSftp(content://com.android.externalstorage.documents/tree/17FB-0715...
  found mount point /mnt/media_rw/17FB-0715 with type vfat

Real FAT32 card:

getFilesystemTimeResolutionForSftp(content://com.android.externalstorage.documents/tree/80AB-022A...
  found mount point /mnt/media_rw/80AB-022A with type sdfat with option fs=vfat:32

The fs=exfat sample in the code comment is from a real exFAT 64GB card.

@wolpi
Copy link
Owner

wolpi commented Jun 23, 2024

I'm really looking forward to that merge script 😄

@lmagyar
Copy link
Contributor Author

lmagyar commented Jun 23, 2024

I'm really looking forward to that merge script 😄

The Python sync? 😄 It is working, I've added remote management (start/stop, with Automate or HomeAssistant) for Tailscale, pFTPd, Termux/sshd services (the later for inotifywait, to catch the changes on the fly). I need a few hours net to finalize it on to a v0.1 quality level, though with a newborn and a relocation I hardly find time to sleep. 😨

I will work on the tech comments above on next week.

@wolpi
Copy link
Owner

wolpi commented Jun 26, 2024

Happy coding between moving boxes and with new born on arm 😄

@lmagyar lmagyar marked this pull request as draft August 30, 2024 18:28
@lmagyar lmagyar changed the title Fix Android bug on FAT32 time resolution and mtime caching WIP: Fix Android bug on FAT32 time resolution and mtime caching Sep 3, 2024
@lmagyar
Copy link
Contributor Author

lmagyar commented Sep 3, 2024

Happy coding between moving boxes and with new born on arm 😄

Yeah, this description is quite accurate. :)

Just a note:

Android inaccurate on file mod. times not only on files we have uploaded, but on eg. new pictures. When we download a fresh new picture through plain-old FS from the SD card, it will have an inaccurate timestamp, but a few hours later it will be rounded to 2s (in case of FAT32) and my sync script will download it again. Not a big issue, doesn't happen too often, but an issue, especially when I delete the downloaded file, and the delete and "modification" causes a conflict.

So this solution should be extended to RoSaf and plain-old FS (when we access files under /fs/storage/XXXX-XXXX). Not a big problem, but it needs some time.

@lmagyar
Copy link
Contributor Author

lmagyar commented Sep 27, 2024

After some "working, but I don't like it" versions, this is the minimalistic version:

  • StorageManagerUtil.getFilesystemTimeResolutionForTreeUri()
    • handles another Android bug (even exFat got a 2s resolution)
    • caches the calculated time resolutions, this wont change while pftpd is running
  • FsFileSystemView, RoSafFileSystemView, SafFileSystemView got a timeResolution argument
    • FsFileSystemView got another safVolumePath argument
    • all got a getCorrectedTime() method
    • but FsFileSystemView uses safVolumePath to use the altered timeResolution only for files under the read-only SAF root path
  • FsFile, RoSafFile, SafFile got a fileSystemView attribute, and ctor argument
    • all uses this to get the proper/corrected time when reading/writing lastModifiedTime
    • ...FtpFile, ...SshFile got a private getFileSystemView() method for casting fileSystemView back to the correct type to minimize the casting
  • all other modifications are only passing the fileSystemView arguments in the ctor-s

Issues:

  • rootFS is not modified, I can't test it, and I assume it is used mainly to access other files, not the SD card
  • this time resolution fixing works only, when the user selected something on the SD card for SAF (the root or subfolder; even the subfolder is not an issue for poFS)
    • but if something else (eg. internal Downloads) is selected, roSAF and SAF will not modify anythng, but poFS won't be able to determine the SD card location in the file system and will not modify any timestamp - I think we can live with it, this is still better than the original Android behaviour

Now if I make new pictures with the camera, stored on the SD card, sync it to the laptop (through poFS, roSAF or SAF), the timestamps will be the correct even seconds immediatelly and not only after a few hours (causing another sync or sync conflict if I deleted them meantime on the laptop).

@lmagyar lmagyar marked this pull request as ready for review September 27, 2024 19:22
@lmagyar lmagyar changed the title WIP: Fix Android bug on FAT32 time resolution and mtime caching Fix Android bug on FAT32/exFAT SD card time resolution and mtime caching Sep 27, 2024
@lmagyar lmagyar marked this pull request as draft September 29, 2024 14:46
@lmagyar
Copy link
Contributor Author

lmagyar commented Sep 29, 2024

Draft again, I will refactor some details (no functional change).

@lmagyar
Copy link
Contributor Author

lmagyar commented Sep 29, 2024

It's ready for review again.

Added caching and made some refactor

The long comment above summarizing the changes got updated!

@lmagyar lmagyar marked this pull request as ready for review September 29, 2024 23:03
@lmagyar
Copy link
Contributor Author

lmagyar commented Oct 5, 2024

Merge conflicts resolved.

@lmagyar
Copy link
Contributor Author

lmagyar commented Oct 5, 2024

OK, conflicts are really fixed now, it builds. :D

Copy link
Owner

@wolpi wolpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole topic is still unbelievable. I would have never put so much effort just in timestamps.

One thing I noticed:
Now Fs..File objects have some relation to SAF. I'm going to add comments about that.

@wolpi wolpi merged commit 9e68949 into wolpi:master Oct 6, 2024
1 check passed
@wolpi wolpi added this to the 7.3 milestone Oct 6, 2024
wolpi added a commit that referenced this pull request Oct 6, 2024
lmagyar pushed a commit to lmagyar/prim-ftpd that referenced this pull request Oct 6, 2024
lmagyar pushed a commit to lmagyar/prim-ftpd that referenced this pull request Oct 6, 2024
@lmagyar lmagyar deleted the pr-fix-time-resolution branch October 6, 2024 11:39
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

Successfully merging this pull request may close these issues.

2 participants