jwiegley / emacs-async

Simple library for asynchronous processing in Emacs
GNU General Public License v3.0
829 stars 68 forks source link

fix: add line buffering logic to process filter #168

Closed Fuco1 closed 1 year ago

Fuco1 commented 1 year ago

Recently I've submitted patch #167 to read messages from child processes.

The way it works is that it reads a base64 encoded "packet", decodes it and if it is an async message it passes it further. However, when the base64 string is too long, process filter can receive chunk of data which is not a complete line and this will break the base64 decoding.

I've added a decorator for the process filter which will buffer the data until a full line is available and only then passes it to the processor. This way you can send really long message packages such as full error backtraces from worker processes to parent.

In case there is some copyright assignment issues, I took the decorator code from my other package sallet so there should be no issue.

thierryvolpiatto commented 1 year ago

Matus Goljer @.***> writes:

Recently I've submitted patch #167 to read messages from child processes.

The way it works is that it reads a base64 encoded "packet", decodes it and if it is an async message it passes it further. However, when the base64 string is too long, process filter can receive chunk of data which is not a complete line and this will break the base64 decoding.

Yes, I thought there was such problem, but as long as the message are short I thought it was enough.

I've added a decorator for the process filter which will buffer the data until a full line is available and only then passes it to the processor. This way you can send really long message packages such as full error backtraces from worker processes to parent.

Generally the way to do this is to advance marker in the process buffer and continue collecting output from it until process finishes. I will merge your code and see later when I will have more time.

Thanks.

In case there is some copyright assignment issues, I took the decorator code from my other package sallet so there should be no issue.

Ok, I have no knowledge on these copyright issues, if someone see a problem please comment. @johnwiegley @stefanmonnier ?

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

You can view, comment on, or merge this pull request online at:

https://github.com/jwiegley/emacs-async/pull/168

Commit Summary

• e3ae4d2 fix: add line buffering logic to process filter

File Changes

(1 file)

• M async.el (22)

Patch Links:

https://github.com/jwiegley/emacs-async/pull/168.patchhttps://github.com/jwiegley/emacs-async/pull/168.diff

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.*Message ID: @.***>

-- Thierry

Fuco1 commented 1 year ago

@thierryvolpiatto

Generally the way to do this is to advance marker in the process buffer and continue collecting output from it until process finishes.

I'm not sure about the part "until process finishes". We specifically want to process the messages as soon as they could be processed to achieve a two-way communication. But you probably mean to "reset" some last-newline-marker every time and then gobble the code up-to there (or some such scheme, I'm sure I could figure it out :D)