Skip to content

lib/fs: Watching is unsupported on android/amd64 (fixes #8709) #8710

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.

Sign up for GitHub

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

Jump to bottom
Merged
calmh merged 1 commit into from
Dec 21, 2022
Merged

lib/fs: Watching is unsupported on android/amd64 (fixes #8709) #8710

calmh merged 1 commit into from
Dec 21, 2022

Conversation

@calmh
Copy link
Member

@calmh calmh commented Dec 8, 2022

Apparently watching on android/amd64 causes a SIGSYS: bad syscall crash. This avoids that.

@calmh calmh mentioned this pull request Dec 8, 2022
Closed
@ghost ghost self-requested a review December 8, 2022 11:01
This was referenced Dec 8, 2022
Closed
Open
@imsodin
Copy link
Member

imsodin commented Dec 8, 2022

@Catfriend1 Could you give your view on this - especially the usability of the app without filesystem notifications and making users aware of that:

Am I somewhat unsure that this is the correct hammer: Are we sure this affects all android/amd64 systems? And also if it ever starts to work again, it means syncthing needs changing. It feels like disabling watching from the android wrapper on affected systems is preferrable, together with a warning to those users that there is no file watching - this means rescans every 1min on android, this will re-ignite the battery drain and everything so slow reports.

@imsodin
Copy link
Member

imsodin commented Dec 8, 2022

Thanks. That makes sense. I still think it would be good to have a warning on the app for those users, but that's obviously not a blocker to doing this fix to get it working at all.

Doesn't the native automatically fall back to rescan 1 m if setting up those watches failed?

As I wrote in the cited comment: It does, and that's what worries me, as it means a lot of fuse activity and thus likely high battery usage.

@imsodin
Copy link
Member

imsodin commented Dec 16, 2022 •
edited
Loading

I am fine with this PR. App changes for those few users affected would be nice but not a necessity in my opinion.

However I just found a link to an issue on fsnotify about the likely root cause - we should at least open a ticket on inotify about it: fsnotify/fsnotify#112
On phone right now thus just dropping the link here.
Never mind - skimming on the phone it looked promising but it's entirely unrelated to the problem at hand.

@calmh calmh merged commit ad0044f into syncthing:main Dec 21, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Dec 21, 2022
@calmh
Merge remote-tracking branch 'upstream/main' into coauthors
2532377 
* upstream/main:
  lib/fs: Watching is unsupported on android/amd64 (fixes syncthing#8709) (syncthing#8710)
  lib/model: Only log at info level if setting change time fails (syncthing#8725)
  lib/model: Don't lower rescan interval from default on auto accepted enc folder (fixes syncthing#8572) (syncthing#8573)
  gui, man, authors: Update docs, translations, and contributors
  gui: Remove unmaintained language variant nl-BE (syncthing#8722)
  gui, script: Fix indentation in lang-en.json to match others (syncthing#8721)
  docker: Ensure entrypoint is executable (syncthing#8719)
  Go 1.19.4
  Update dependencies (syncthing#8717)
  gui, man, authors: Update docs, translations, and contributors
@calmh calmh added this to the v1.22.3 milestone Dec 27, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Jan 23, 2023
@calmh
Merge branch 'main' into dockerpool
f8b7cc4 
* main: (69 commits)
  Handle relay connect timeout (fixes syncthing#8749) (syncthing#8755)
  gui, man, authors: Update docs, translations, and contributors
  build: Go 1.19.5
  gui, man, authors: Update docs, translations, and contributors
  script: Add weblatedl.go for downloading updated translations (syncthing#8723)
  gui: Allow to translate action and type in Recent Changes modal (syncthing#8548)
  gui, man, authors: Update docs, translations, and contributors
  gui: Fix undefined lastSeenDays error in disconnected-inactive status check (ref syncthing#8530) (syncthing#8730)
  gui, man, authors: Update docs, translations, and contributors
  gui, api: Indicate running under container (syncthing#8728)
  lib/fs: Use io/fs errors as recommended in std lib (syncthing#8726)
  build: Handle co-authors (ref syncthing#3744) (syncthing#8708)
  lib/fs: Watching is unsupported on android/amd64 (fixes syncthing#8709) (syncthing#8710)
  lib/model: Only log at info level if setting change time fails (syncthing#8725)
  lib/model: Don't lower rescan interval from default on auto accepted enc folder (fixes syncthing#8572) (syncthing#8573)
  gui, man, authors: Update docs, translations, and contributors
  gui: Remove unmaintained language variant nl-BE (syncthing#8722)
  gui, script: Fix indentation in lang-en.json to match others (syncthing#8721)
  docker: Ensure entrypoint is executable (syncthing#8719)
  Go 1.19.4
  ...
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Dec 22, 2023
@syncthing syncthing locked and limited conversation to collaborators Dec 22, 2023
@calmh calmh deleted the noandroidnotify branch May 26, 2025 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion

Projects

None yet

Milestone

v1.23.0

Development

Successfully merging this pull request may close these issues.

3 participants

@calmh @imsodin @st-review