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

Disable missing syscall on android-amd64, support AVD 12+ and Chromebook arch x86_64 #8709

Closed
wants to merge 2 commits into from

Conversation

Catfriend1
Copy link
Contributor

@Catfriend1 Catfriend1 commented Dec 7, 2022

Purpose

Without this PR, "libsyncthing.so" (SyncthingNative) crashes with "bad syscall error".
ref: Catfriend1/syncthing-android#583 (comment)

Affected: Both Syncthing(Android) and Syncthing-Fork (for Android). The missing syscall is called by "unix.EpollWait" from the god module "github.com/syncthing/notify". "unix.EpollWait" is part of "golang.org/x/sys". Upgrading "golang.org/x/sys" to the most recent version 0.3.0 doesn't solve the problem. So I've made this PR.

Issue: syncthing/syncthing-android#1703

Testing

Verified on Android AVD 13. The disabled syscall (unix.epollwait) which isn't available on android-amd64 arch allows SyncthingNative to run without the file watcher. Platforms using this arch are for example: Android emulator >= 12, Google Chromebook. The arch is also labeled "x86_64" in app/src/jniLibs.

Screenshots

image

Documentation

ref: Catfriend1/syncthing-android#583

Please go light on me, I don't know much about the go stuff and appreciate any help on this. My main goal isn't dealing with the affected go lib or syscall stuff but I really need my Android emulator be able to execute the Syncthing Android app to continue my work on newer API levels and Android releases 12+.

Authorship

Catfriend1

@Catfriend1 Catfriend1 added the bug A problem with current functionality, as opposed to missing functionality (enhancement) label Dec 7, 2022
@Catfriend1 Catfriend1 self-assigned this Dec 7, 2022
@Catfriend1 Catfriend1 changed the title Disable missing syscall on android-amd64 (fixes #https://github.com/Catfriend1/syncthing-android/issues/583) Disable missing syscall on android-amd64, support AVD 12+ and Chromebook arch x86_64 Dec 7, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Dec 8, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Dec 8, 2022
@calmh
Copy link
Member

calmh commented Dec 8, 2022

Could you give #8710 a spin, which I think is a cleaner way to disable the watcher on android/amd64?

@imsodin
Copy link
Member

imsodin commented Dec 8, 2022

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.

@bt90
Copy link
Contributor

bt90 commented Dec 8, 2022

Can we check if epollWait is implemented without triggering a crash? That would be the cleanest approach IMHO.

@Catfriend1
Copy link
Contributor Author

Can we check if epollWait is implemented without triggering a crash? That would be the cleanest approach IMHO.

I don't know if this is possible in Go. I've tracked the crash down this way:

basicfs_watch.go > gomod/notify > gomod/x/sys (provides unix.epollwait)

Another dirty workaround I've tried out yesterday is blanking the function "loop" in gomod/notify as that's the place where unix.epollwait is called. But without it, there would be no watching and that's why we are currently discussing disabling the basicfs_watch capability on this rarely used platform.

@Catfriend1
Copy link
Contributor Author

Superseded by #8710

@Catfriend1 Catfriend1 closed this Dec 8, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Dec 21, 2022
* 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
@Catfriend1 Catfriend1 deleted the catfriend1-workaroundInotifyAndroidAmd64 branch January 5, 2023 19:52
calmh added a commit to calmh/syncthing that referenced this pull request Jan 23, 2023
* 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 8, 2023
@syncthing syncthing locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants