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

termux-api command should detect if Termux:API plugin is not installed and print error. #200

Open
twaik opened this issue Feb 7, 2025 · 14 comments · May be fixed by #201
Open

termux-api command should detect if Termux:API plugin is not installed and print error. #200

twaik opened this issue Feb 7, 2025 · 14 comments · May be fixed by #201

Comments

@twaik
Copy link
Member

twaik commented Feb 7, 2025

Feature description
You can invoke command like echo $(pm path com.termux.api 2>&1 </dev/null) in termux to get termux:api plugin apk path or get error otherwise, it should be usable to detect plugin installation even without PackageManager API (or JVM).

Background information
Termux:X11 companion package can detect if main APK is installed or not via PackageManager API. Termux:API should do that too.
There are enough people coming to discord, matrix and reddit complaining about termux-api-based commands are hanging indefinitely.

Redirecting everything to /dev/null or anonymous pipes is necessary in command echo $(pm path com.termux.api 2>&1 </dev/null), the same applies to the case if you are porting it to C.

Related to:
#138
#153
#80

@agnostic-apollo
Copy link
Member

You are forgetting the delay an external call like this will cause. Anyways, this is already planned to be fixed with termux-am-socket updates.

@twaik
Copy link
Member Author

twaik commented Feb 7, 2025

pm is a simple command, it does not invoke JVM. AFAIK it invokes only a few Binder calls to system server so it should not give any noticeable delay.

@agnostic-apollo
Copy link
Member

There is a 20ms delay even on a snapdragon 865.

@twaik
Copy link
Member Author

twaik commented Feb 7, 2025

Not really noticeable. And the result can be cached for the case of batch api calls.

@agnostic-apollo
Copy link
Member

Every delay matters as it adds up in sequential calls. And when certain API calls themselves take 25-30ms, then 20ms is hell of a lot of delay.

There are better solutions for this case where app manages the state. As for other general caching for user scripts, read https://github.com/termux/termux-core-package/blob/0.1.0-prerelease/site/pages/en/projects/docs/usage/utils/termux-apps-info-app-version-name.md#rationale-and-design

@twaik
Copy link
Member Author

twaik commented Feb 7, 2025

I mean you can cache pm response somewhere to $PREFIX/tmp or $PREFIX/var/run and not invoke it again at the same second/minute/hour/whatever.
The most valuable point in this case is to print an error in the case if app not installed and not leave end users in situation where they must guess or google what is wrong and why nothing works and just hangs.

@agnostic-apollo
Copy link
Member

A user can uninstall the app after caching and you will get wrong state on later calls. Read the relevant links, and there is a better solution for this case like I said, wait for it to be implemented.

@twaik
Copy link
Member Author

twaik commented Feb 7, 2025

The doc is long. And as far as I understand it it describes saving some data to environment variables which will not work in this case since you can not save data to environment variables of parent process.

Also it is possible to make transmit_stdin_to_socket function implement some timer which will check if app is installed or not in the case if there was no pending data in a second or so. So there will be no latency in the case if plugin is installed and responds to broadcast.

@agnostic-apollo
Copy link
Member

App will save to env file and update it when any plugin app state changes. Scripts can source the file, parent process doesn't matter, and sourcing is an internal call in shell and is instant.

There is no need for messing around with api scripts for this, am-socket can check if target app is installed before sending intent, that will be much faster, likely instant.

@twaik
Copy link
Member Author

twaik commented Feb 7, 2025

The solution you suggest will work with 0.119+ but will not work for 0.118 or outdated forks like termux-monet (afaik some people still use it).

My suggestion should work with any existing termux-app version and should not be too much complicated.

int transmit_socket_to_stdout(int input_socket_fd) {
	...
	struct pollfd pfd = { .fd = input_socket_fd, .events = POLLIN | POLLOUT | POLLERR | POLLHUP | ... anything else ... };
	if (poll(&pfd, 1, 1000) == 0) {
                // Timeout, we should check if Termux:API app is installed
                system("echo $(pm path com.termux.api 2>&1 </dev/null | grep com.termux.api > /dev/null || echo \"Termux:API plugin is not installed. You should download and install apk from <github or f-droid link>... \" >2 )");
                // This or similar code in C which will invoke `pm` and print error.
                exit(1); // to avoid hanging, should be called only when app is not installed.
	}
	...
	while ((len = recvmsg(input_socket_fd, &msg, 0)) > 0) {

It implements simple timeout which should not add any delay (maybe a few nanosecs for a pollfd syscall), does not complicate the function much and does not change function logic. It will simply invoke the check in the case if Termux:API plugin did not respond.

@agnostic-apollo
Copy link
Member

Timeouts is not possible, as termux-api app may connect or respond after a long time, like for location APIs.

Its not my responsibility to support older app versions considering that both android 5 and 7 users can use latest versions. Fixing termux-am-socket will also fix issues for all intents, not just temrux-api, so a better and faster solution.

@twaik
Copy link
Member Author

twaik commented Feb 8, 2025

Nobody says it should terminate the process in the case if there is no output.
As you said the code checking if Termux:API app is installed can cause a delay comparable to the API call delay.
My suggestion is to perform a check only in the case if the pipe/socket was idle for too long. Nobody will be hurt if the process being idle for more than 1000 ms will spend 20 ms to check if the app is installed or not. And I did not say the whole process should be killed after a timeout. It should happen only in the case if API app is not installed.

@agnostic-apollo
Copy link
Member

The delay exists if external call in shell/binary is made, plus lag due to binder shenanigans. If done in app, then there won't be a delay as Android caches package/application info states on its own using PropertyInvalidatedCache with the cache_key.package_info property.

Additionally, you are misunderstanding how temrux-api.c works, the waiting for app happens on blocking accept() call, not when waiting for stdout. You could possibly use select() with a timeout beforehand, but that can create race conditions.

@twaik
Copy link
Member Author

twaik commented Feb 8, 2025

Additionally, you are misunderstanding how temrux-api.c works, the waiting for app happens on blocking accept() call, not when waiting for stdout

You are right. We should poll input_server_socket for POLLIN events before accept call.

but that can create race conditions

The program creates sockets with unique uuids for each invocation so should be fine. Otherwise we can use lock files to prevent concurrent access.

The delay exists if external call in shell/binary is made, plus lag due to binder shenanigans

As I said before, the check should not be triggered if app actually responds. And the check will be performed only in the case if the program is idle for too long. If you still think it will create a delay we can do some other thing like creating global volatile bool connectionPerformed = false; variable which will be set to true right after blocking accept call and new pthread which will wait 1000 ms and check if connectionPerformed is still false and perform a check otherwise. The check will shutdown the whole process only in the case if API app is not installed so it should not be a 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

Successfully merging a pull request may close this issue.

2 participants