Skip to content

#6258 Switch legacy signal() with sigaction() - #6372

Open
akleshchev wants to merge 4 commits into
developfrom
andreyk/viewer_6258_3
Open

akleshchev wants to merge 4 commits into
developfrom
andreyk/viewer_6258_3

Conversation

@akleshchev

@akleshchev akleshchev commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Swap signal() with sigaction()

Due to a reported crash that wasn't reported to bugsplat, logs:

2026-09-24T16:02:07Z INFO #WebSocket# llcorehttp/llwebsocketmgr.cpp(252) start : Starting WebSocket server on port 9020 (localhost only)
2026-09-24T16:02:07Z WARNING #LLProcess# llcommon/llprocess.cpp(718) LLProcess::launch : Could not locate 'code' on PATH -- launch will likely fail
2026-09-24T16:02:07Z WARNING #LLProcess# llcommon/llprocess.cpp(559) LLProcess::create : Failed to create process: failed to create process: failed to launch code: default_launcher: No such file or directory [system:2 at /Users/runner/work/viewer/viewer/build-darwin-universal/packages/include/boost/process/v2/posix/default_launcher.hpp:409:78 in function 'basic_process<Executor> boost::process::posix::default_launcher::operator()(Executor, error_code &, const typename std::enable_if<net::execution::is_executor<Executor>::value || net::is_executor<Executor>::value, filesystem::path>::type &, Args &&, Inits &&...) [Executor = boost::asio::any_io_executor, Args = std::vector<std::string> &, Inits = <boost::process::process_stdio &>]']
2026-09-24T16:02:07Z WARNING #ScriptEditorWS# newview/llscripteditorws.cpp(402) launchVSCode : Failed to launch VS Code. Ensure the 'code' command is available on your PATH.
2026-09-24T16:02:07Z WARNING # newview/llappviewer.cpp(1403) doFrame :  Someone took over my signal/exception handler (post messagehandling)!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The process-wide SIGPIPE disposition can overwrite the application's crash handler.

Review effort: Lite
Findings: None

What changed in this PR

This pull request replaces legacy signal() handling with sigaction() for SIGPIPE during process launches.

Changes:

  • Configures SIGPIPE handling with sigaction().
  • Preserves existing process-launch behavior.
File Summary
indra/​llcommon/​llprocess.cpp Replaces signal() with sigaction(), but still changes the process-wide SIGPIPE disposition.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved SIGPIPE lifetime and sigaction() error-handling issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)

Comment thread indra/llcommon/llprocess.cpp Outdated
Comment thread indra/llcommon/llprocess.cpp Outdated
@akleshchev
akleshchev force-pushed the andreyk/viewer_6258_3 branch 2 times, most recently from bc4c250 to 1151754 Compare September 24, 2026 21:29
@akleshchev
akleshchev requested a lite review from Copilot September 24, 2026 21:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

One-time initialization can leave SIGPIPE assigned to the crash handler on later launches.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread indra/llcommon/llprocess.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical issues remain around deferred SIGPIPE protection and the missing direct <mutex> include.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity

Open (3)

Comment thread indra/llcommon/llapp.cpp Outdated
Comment thread indra/llcommon/llprocess.cpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: akleshchev <117672381+akleshchev@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
✅ (akleshchev)[https://github.com/akleshchev]
❌ @Copilot
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants