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

Using multiprocessing.Queue does not work on Android #271

Closed
fornwall opened this issue Mar 10, 2018 · 10 comments
Closed

Using multiprocessing.Queue does not work on Android #271

fornwall opened this issue Mar 10, 2018 · 10 comments

Comments

@fornwall
Copy link
Contributor

Trying to use multiprocessing.Queue (which asciinema v2 started to do) on a python compiled for Android does not work:

ImportError: This platform lacks a functioning sem_open implementation, therefore, the required synchronization primitives needed will not function, see issue 3770.

The linked issue is this: https://bugs.python.org/issue3770

While python on Android is a niche OS, there is quite some interest in asciinema from Termux users. So I'm wondering if you're interested in replacing multiprocessing.Queue (at least as an alternative) to make asciinema work there?

@ku1ik
Copy link
Contributor

ku1ik commented Mar 11, 2018

Hey. Good catch. I'm happy to make it work under Python versions without support for multiprocessing.

I'd still like to use multiprocessing by default, and fallback to alternative when it's not available.

Maybe it would be possible to use queue.Queue + threading.Thread instead of multiprocessing.Queue and multiprocessing.Process, I think these quack the same way.

@ku1ik
Copy link
Contributor

ku1ik commented Mar 11, 2018

Try replacing this line: https://github.com/asciinema/asciinema/blob/develop/asciinema/asciicast/v2.py#L7 with this:

try:
    from multiprocessing import Process, Queue
except ImportError:
    from threading import Thread as Process
    from queue import Queue

I've tested it on my laptop, by adding a typo to the multiprocessing import to trigger ImportError. It used threading and recorded asciicast with success.

If you can verify this solves it for Android/Termux feel free to send me PR or, just comment here and I'll commit that patch.

@fornwall
Copy link
Contributor Author

fornwall commented Apr 6, 2018

Importing multiprocessing.Process and multiprocessing.Queue does not cause an ImportError, that happens later when one tries to use Queue(). However, importing multiprocessing.synchronize does cause an ImportError directly, so I tried out the following patch:

diff -u -r ../asciinema-2.0.1/asciinema/asciicast/v2.py ./asciinema/asciicast/v2.py
--- ../asciinema-2.0.1/asciinema/asciicast/v2.py>       2018-04-04 09:05:41.000000000 +0200
+++ ./asciinema/asciicast/v2.py>2018-04-06 23:24:44.432657505 +0200
@@ -4,7 +4,16 @@
 import json.decoder
 import time
 import codecs
-from multiprocessing import Process, Queue
+
+try:
+    # Importing synchronize is to detect platforms where
+    # multiprocessing does not work (python issue 3770)
+    # and cause an ImportError. Otherwise it will happen
+    # later when trying to use Queue().
+    from multiprocessing import synchronize, Process, Queue
+except ImportError:
+    from threading import Thread as Process
+    from queue import Queue
·
 from asciinema.pty_recorder import PtyRecorder

It works somewhat: A recording works when exiting the shell with exit, but using Ctrl+D does not work - I guess due to signal handling working differently when not using multiple processes? Do you have an idea what to try out or look at for that?

@ku1ik
Copy link
Contributor

ku1ik commented Apr 7, 2018

Ok, great, then we're one step closer 👍

Not sure why Ctrl+D doesn't work with this...

@ku1ik
Copy link
Contributor

ku1ik commented Apr 7, 2018

If I remember correctly I tested it on macOS by replacing processing code with threading code, and Ctrl+D worked fine, so it may be Android specific thing.

@jfhc
Copy link

jfhc commented Apr 7, 2018

I am running the Termux Android app on a Chromebook. I tried @fornwall's patch and it got asciinema to run for me (woohoo!). Ctrl+D works for me too on my Chromebook.

When I run it in Termux on my phone (Nokia 6, Android 7.1.2) the same thing happens, but I also found Ctrl+d didn't work using the CodeBoard keyboard. But then I tried ctrl inputs for other things (tried it with tmux) but it didn't work for that either, so I wonder if it's an issue with CodeBoard or something - either way, doesn't seem to be an asciinema issue. @fornwall what keyboard are you using, and ctrl+... work with other terminal programs in Termux?

@fornwall
Copy link
Contributor Author

fornwall commented Apr 8, 2018

@jfhc Thanks for the testing!

After testing on more devices the Ctrl+D issue seems to be device specific (perhaps with Android 8.0 or later?). I created #295 for that issue.

That issue does not depend on the multiprocessing fallback we're discussing here (asciinema 1.4 had the same problem), so I created PR #294 to merge the proposed patch. Nice work @sickill !

@ku1ik ku1ik closed this as completed in #294 Apr 9, 2018
ku1ik pushed a commit that referenced this issue Apr 9, 2018
On platforms (Android) where multiprocessing is not supported we now
use threads instead. Fixes #271.
@ku1ik
Copy link
Contributor

ku1ik commented Apr 9, 2018

Good job on testing Ctrl+D 👍

@Hotschmoe
Copy link

Would this process for creating a multiprocessing fallback work for ansible? Ansible is currently crashing on Termux thanks to this multiprocessing lacking from android. Issue here: termux/termux-packages#1815

@pwoolvett
Copy link

pwoolvett commented Sep 7, 2019

Note: python3.6.8 (via userland, debian) wont raise ImportError. Manually instantiating a Queue raises PermissionError.

adding Queue() inside the try block and also excepting PermissionError to the fix in #271 (comment) solved the problem.

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

5 participants