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.

Sign up for GitHub

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

Jump to bottom

Nailgun disconnection due to pantsd panic #11926

Closed
stuhood opened this issue Apr 15, 2021 · 3 comments · Fixed by #12152
Closed

Nailgun disconnection due to pantsd panic #11926

stuhood opened this issue Apr 15, 2021 · 3 comments · Fixed by #12152
Assignees
@jsirois
Labels
bug

Comments

@stuhood
Copy link
Member

stuhood commented Apr 15, 2021

Unfortunately we still see a variant of #11364, but after #11731, it exhibits as:

thread panicked while processing panic. aborting.

...rendered in the .pants.d/pants.log, while the client sees:

native_engine.NailgunClientException: Nailgun client error: "Nailgun client error: Client exited before the server\'s result could be returned."
@stuhood stuhood added the bug label Apr 15, 2021
@stuhood stuhood mentioned this issue Apr 15, 2021
Closed
This was referenced May 11, 2021
Merged
Merged
Eric-Arellano added a commit that referenced this issue May 11, 2021
@Eric-Arellano
[internal] Always upload Pants log in CI (#12050)
58f3f33 
It's not being uploaded on CI failures, even though that is often the most useful time for the log, e.g. to debug #11926
@Eric-Arellano
Copy link
Contributor

Note, several users have reported seeing this native_engine.NailgunClientException error in CI, especially when using remote caching but not exclusively. Pantsbuild/pants has seen the error multiple times - although, the pants.log has not included the line "thread panicked while processing panic", and we instead suspect that the failures were from being OOM-killed.

The NailgunClientException (soon to be PantsdClientException as of #12107) is the generic exception for Pantsd being killed during a Pants run. This issue is instead focused on the edge case of that happening via thread panicked while processing panic..

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented May 28, 2021 •
edited
Loading

This happened in the integration test TestPantsDaemonIntegration.test_unhandled_exceptions_only_log_exceptions_once. That could be a good angle to reproduce this, run that test enough times with --force. https://github.com/pantsbuild/pants/pull/12140/checks?check_run_id=2696289466#step:11:478

Update: running it 200 times in Linux did not reproduce.

@jsirois
Copy link
Contributor

jsirois commented May 30, 2021 •
edited
Loading

I think I've found this. Key note is the original issue as reported in #11364 was on Dec 22, 2020. The apparently problematic change was submitted on Dec 3rd 2020 and 1st released in 2.2.0.dev1. I've commented on the bug in that PR: https://github.com/pantsbuild/pants/pull/11262/files#r642013320

jsirois added a commit to jsirois/pants that referenced this issue May 30, 2021
@jsirois
Fix Key interning race.
b42580d 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in key_get. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

[ci skip-build-wheels]
@jsirois jsirois mentioned this issue May 30, 2021
Merged
@jsirois jsirois self-assigned this May 30, 2021
jsirois added a commit to jsirois/pants that referenced this issue May 30, 2021
@jsirois
Fix Key interning race.
82ccb94 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in key_get. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

[ci skip-build-wheels]
jsirois added a commit that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (#12152)
a59be85 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes #11926
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
b036b34 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@jsirois jsirois mentioned this issue Jun 1, 2021
Merged
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
ec1ea86 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
@jsirois jsirois mentioned this issue Jun 1, 2021
Merged
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
31bd58a 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
@jsirois jsirois mentioned this issue Jun 1, 2021
Merged
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
d09a189 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
@jsirois jsirois mentioned this issue Jun 1, 2021
Merged
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
2ee6fec 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
5c79b42 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
jsirois added a commit that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (#12152) (#12158)
519d2bd 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes #11926

(cherry picked from commit a59be85)
jsirois added a commit that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (#12152) (#12159)
f53a20c 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes #11926

(cherry picked from commit a59be85)
jsirois added a commit to jsirois/pants that referenced this issue Jun 1, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
d261d2f 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
jsirois added a commit that referenced this issue Jun 2, 2021
@jsirois
Fix Key interning race. (#12152) (#12160)
cdf0bc1 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes #11926

(cherry picked from commit a59be85)
jsirois added a commit to jsirois/pants that referenced this issue Jun 2, 2021
@jsirois
Fix Key interning race. (pantsbuild#12152)
d0e26b2 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes pantsbuild#11926

(cherry picked from commit a59be85)

[ci skip-build-wheels]
jsirois added a commit that referenced this issue Jun 2, 2021
@jsirois
Fix Key interning race. (#12152) (#12161)
70a9527 
Previously a `Key` could be observed before it was added to the
`reverse_keys` mapping leading to a panic in `key_get`. That panic used
a debug format on the problematic key and the `Debug` impl for `Key`
indirectly uses `key_get`. This results in a panic while handling a
panic.

Fix the root cause of the panic, ensuring the panic can never happen
as was intended. Also fix the recursive use of `key_get` in the panic
for sanity sake.

Fixes #11926

(cherry picked from commit a59be85)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsirois jsirois

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Key interning race. jsirois/pants
3 participants
@stuhood @jsirois @Eric-Arellano