On 15 October 2013 03:15, Dimitar Zhekov <dimitar.zhekov@gmail.com> wrote:

Hi,

I'm looking how to fix this famous bug without too many changes, and
found something strange: tools.c runs it's command with spawn async,
but uses blocking I/O to read the data from stdout/stderr.

Given that it sets up io channels and callbacks when data is available I suspect that the writer thought they were using asynchronous i/o. ;)

Any idea why is that? What comes to ming is blocking Geany until the
command finishes, to be sure that the current document still exists and
it's selection is unchanged, and thus can be safely replaced with the
command output.

This would be a correct reason to block Geany, but deliberately, not by accident.

However, if we block reading stderr on Windows, and the command outputs
4KB (or whatever is the limit) to stdout, that'll cause a deadlock, as
evident by the previous messages in this thread. Same if reading stdout
and the command produces a lot of stderr, though that's unlikely.

Its likely to do the same on Linux, but IIUC the limit is larger these days, 64k = 16 * PIPE_BUF. (The 16 is a #define in the kernal)

I can emulate the (non-)blocking behaviour, but am not sure if this
flag currently has any effect under Windows, considering it's buggy
implementation of g_io_add_watch and g_io_channel_set_flags for
pipes (which are the problem to fix in the first place).

Or possibly try the more complicated GIO g_input_stream_read_async() and friends and see if it works on windows.

If tools_execute_custom_command really wants to block Geany, the right
thing is to spawn the command, and then cycle reading stderr and
stdout asynchronously until the command completes, instead of adding
I/O watches to the message loop just to block it.

The current loop that writes the selection to stdin seems capable of
causing a deadlock under Windows too, if the command generates 4KB
output and stops before the full selection is written, since we use
blocking I/O to write to stdin...

And not just on windows, if the selection is a whole file it can easily exceed the Linux limit of 64k.

Basically the whole spawning thing needs to be looked at critically, especially those uses that do i/o to the child process. A working version of each permutation sync/async, reads/writes/both needs to be put in utils and all of Geany should then use that implementation and the callbacks use known good idioms.

Cheers

Lex

--
E-gards: Jimmy
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel