go-exec/module: Run the prompts callbacks in the main thread - #1734
Open
3v1n0 wants to merge 9 commits into
Open
go-exec/module: Run the prompts callbacks in the main thread#17343v1n0 wants to merge 9 commits into
3v1n0 wants to merge 9 commits into
Conversation
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
3 times, most recently
from
July 15, 2026 09:57
8332d25 to
862f177
Compare
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
3 times, most recently
from
July 15, 2026 10:08
9ec17bf to
9e4cbfb
Compare
3v1n0
added
the
e2e-tests
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the pam/go-exec PAM module to ensure PAM conversation callbacks (notably pam_prompt() and pam_error()) are executed on the calling thread by running a dedicated GMainContext/GMainLoop in that thread and dispatching the prompt/error work onto it. This addresses thread-safety issues and avoids deadlocks in callers that expect conversation callbacks to run on their own thread (e.g., Qt-based PAM workers).
Changes:
- Introduces a per-call “action context” main loop in the calling thread and uses
g_main_context_invoke_full()to runpam_prompt()/pam_error()on that thread. - Refactors state to carry
pam_handle_t *and the newaction_contextinActionData(instead ofModuleData) to support cross-thread invocation safely. - Adjusts D-Bus
Prompthandling to return asynchronously once the main-thread prompt completes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
from
July 15, 2026 21:55
9e4cbfb to
914b03a
Compare
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
7 times, most recently
from
July 22, 2026 15:31
a08cbd7 to
da936bf
Compare
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
from
July 24, 2026 21:15
da936bf to
11dea4f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1734 +/- ##
==========================================
- Coverage 88.01% 87.94% -0.07%
==========================================
Files 96 96
Lines 7016 7093 +77
Branches 112 128 +16
==========================================
+ Hits 6175 6238 +63
- Misses 785 786 +1
- Partials 56 69 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
from
July 24, 2026 21:29
11dea4f to
28788b5
Compare
pam_prompt leads to calling code into the caller application, and doing this in a different thread may be unsafe. So run a separated context/loop in the main thread and use it to call code that should run in the main thread
While it's a per-module information there's no need to save it there since it's something that will be available for each action, and this would allow further simplifications
We may want to notify this in the main thread too, so let's prepare the code for that
pam_error implies calling conversation functions in the calling application, so do it in the same thread.
While this is not fully required because g_dbus_method_invocation_return_value() is thread safe in this context, it's still better to use it in the threads it belongs.
As pam_error() is just a definition for pam_prompt(), we can unify the code to run them in the main action thread. Add an helper to use conversations from the model in the main thread and share the same code for all the conversations we trigger.
Ideally the last changes should make the conversation handling safer and they seem to fix the issues when running in a local s390x container, but we may still get failures. So let's just skip the tests for the arch that seems to be problematic Related to: canonical#966
3v1n0
force-pushed
the
run-prompts-in-action-context
branch
from
July 24, 2026 21:42
28788b5 to
1ecdbdb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pam_prompt leads to calling code into the caller application, and doing this in a different thread may be unsafe.
So run a separated context/loop in the main thread and use it to call code that should run in the main thread
Superseeds #1726 (/cc @adombeck)
UDENG-11102