Skip to content

Conversation

@fourteatoo
Copy link

Signals were not delivered unless the input pipe was kept open by the elixir program.
The design is still broken (signals still travel synchronously with the data stream) but for most purposes this patch will alleviate the problem.

Otherwise we can't send signals to the controlled process.
@alco
Copy link
Owner

alco commented Dec 8, 2015

Hi. Thanks for investigating this! I would like to think some more about it and clean up the way we're working with pipes between Elixir<-->goon and between goon<-->external process to make it less hacky. I'll push something later this week.

Any suggestions on how to make signal delivery more reliable in the presence of data that is constantly being fed to the external process? What I think we should do is slice the data into small chucks in Porcelain before sending it to the port. That way we would at least have a minimum guaranteed latency before a signal can be put on the pipe the goes from Elixir to goon.

@alco
Copy link
Owner

alco commented Dec 8, 2015

Em, disregard what I said about latency above. To put it more accurately, slicing the data will allow us to reduce the average latency for signal delivery.

@fourteatoo
Copy link
Author

Unless I'm missing something, slicing the data won't buy you much. AFAIK if
the process stops reading its stdin goon will hang: no output, no signals.
A better solution would be to transmit the signals on a different file
descriptor and have them dealt with by a separate thread.

On Tue, 8 Dec 2015, 21:27 Alexei Sholik notifications@github.com wrote:

Em, disregard what I said about latency above. To put it more accurately,
slicing the data will allow us to reduce the average latency for signal
delivery.


Reply to this email directly or view it on GitHub
#6 (comment).

@liveforeverx
Copy link

Hi, @alco, do you still plan to build some solution for 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

Successfully merging this pull request may close these issues.

3 participants