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: responce was interpritated as function call #40

Closed
wants to merge 6 commits into from
Closed

Fix: responce was interpritated as function call #40

wants to merge 6 commits into from

Conversation

Examnes
Copy link

@Examnes Examnes commented Sep 16, 2023

This pull request fixes a bug I encountered when using the RPC library on multiple machines. An attempt to create a function handler on both machines caused the response containing the result of the function to be interpreted as a request to execute the function, which subsequently caused the function handler to send a "function unavailable" response, which was also interpreted as a function call on the other side. Ultimately, calling any function with this configuration resulted in an infinite loop.

@AmandaCameron
Copy link
Collaborator

I might not be awake enough for this yet, but it seems this will not actually fix it. "function unavailable" (or any other error return, or rpc method returning two values) will match the same 3-entry check you're adding, so it'd still loop. Were both hosts OpenOS? I can't say I ever noticed this when using it for my base mainframe's rpc protocol, but I also did the server impl myself in my base mainframe program.

I've been meaning to poke @XeonSquared about the slight inconsistency that failures return false, foo, but successes only return foo, making it possible for a rpc that returns false, foo to be interpreted as an RPC failure, though

@XeonSquared
Copy link
Member

Got stuck into this today to figure out what was going on - turns out you can trigger it by setting up two OpenOS hosts and doing exportfs /, then trying to mount one from the other. Oops.

So I implemented some type checking, which solved the original issue, and did some other cleanup, plus improved error propagation like @AmandaCameron suggested. This did mean changing the protocol slightly - when a function executes correctly, the true from pcall is preserved and passed to the client also, allowing client software to differentiate between timeouts, the function not existing, and the function erroring.

I've also updated the PsychOS version to match, not that it matters to anyone other than me and maybe AmandaC...
Anyway, I'm pretty sure it works, but I wouldn't complain about other people testing.

@Examnes Examnes closed this Sep 25, 2023
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.

None yet

3 participants