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

Backport console fixes #65

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 13, 2024

This backports a couple of fixes for console-related hangs/deadlocks.

dscho and others added 5 commits February 13, 2024 16:05
The `bfd.h` header file that is included in `binutils` has the line
`#include "ansidecl.h"`, which is fragile because it prefers Cygwin's
`include/ansidecl.h` (as opposed to `#include <ansidecl.h>`, which would
only look in the system include paths).

This matters because as of v2.42, `bfd.h` also makes use of the
`ATTRIBUTE_WARN_UNUSED_RESULT` macro.

So let's just copy that macro (and while at it, the other `ATTRIBUTE_*`
macros) from binutils' `ansidecl.h` file, to avoid compile errors while
compiling `dumper.o` that look like this:

  /usr/include/bfd.h:2770:1: error: expected initializer before ‘ATTRIBUTE_WARN_UNUSED_RESULT’
   2770 | ATTRIBUTE_WARN_UNUSED_RESULT;
        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If disable_master_thread flag is set between the code checking that
flag not be set and the code acquiring input_mutex, input record is
processed once after setting disable_master_thread flag. This patch
prevents that.

Fixes: d4aacd5 ("Cygwin: console: Add missing input_mutex guard.")
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If non-cygwin process is started in pty, closing from_master_nat
pipe handle was missing in fhandler_pty_slave::input_transfer().
This occured because the handle was duplicated but not closed.

msys2/msys2-runtime#198

Fixes: 29431fc ("Cygwin: pty: Inherit typeahead data between two input pipes.")
Reported-by: Hakkin Lain
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In pty master_thread, 6 handles are duplicated when CallNamedPipe()
requests that. Though some of them are not used so should be closed,
they were not. This causes handle leak potentially.

Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Previously, VMIN and VTIME did not work at all. This patch fixes that.

Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho self-assigned this Feb 13, 2024
@dscho dscho merged commit 87d5722 into git-for-windows:main Feb 14, 2024
2 checks passed
@dscho dscho deleted the backport-console-fixes branch February 14, 2024 19:44
@dscho
Copy link
Member Author

dscho commented Feb 14, 2024

I merged this (and opened the corresponding PR at git-for-windows/MSYS2-packages#158) so that it would get into Git for Windows v2.44.0-rc1, still.

@dscho
Copy link
Member Author

dscho commented Feb 15, 2024

I fear that this might be the culprit behind the 3 failed attempts to run ci-artifacts successfully (latest failed attempt). This is a flaky failure, though, as the fourth attempt succeeded... sigh

@dscho
Copy link
Member Author

dscho commented Feb 20, 2024

I fear that this might be the culprit behind the 3 failed attempts to run ci-artifacts successfully (latest failed attempt). This is a flaky failure, though, as the fourth attempt succeeded... sigh

Actually no, I was wrong, the flaky behavior happened before the MSYS2 runtime was upgraded. I opened a ticket here: git-for-windows/git-sdk-64#81

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.

2 participants