Bug 1833756 - Crash when creating a new terminal over D-Bus and immediately using the GSimpleAction to create a second one
Summary: Crash when creating a new terminal over D-Bus and immediately using the GSimp...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: gnome-terminal
Version: 33
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Debarshi Ray
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1882972 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-05-10 10:01 UTC by Christian Persch
Modified: 2021-05-31 15:17 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-05-31 15:17:50 UTC
Type: Bug


Attachments (Terms of Use)

Description Christian Persch 2020-05-10 10:01:49 UTC
Description of problem:

Crash, likely due to fedora patch. 

Version-Release number of selected component (if applicable):
0:3.36.1.1-1.fc32

How reproducible:
Unknown.

Steps to Reproduce:
N/A

Actual results:
Crrash.

Expected results:
No crash.

Additional info:

Backtrace at https://retrace.fedoraproject.org/faf/reports/2887474/ shows terminal_screen_reexec_from_screen_with_override_command produces a SEGV. This function is only present in the fedora patch, not upstream.

My first guess would be that parent_screen->priv->exec_data is NULL when it's being dereferenced.

Comment 1 Fedora Program Management 2021-04-29 16:25:01 UTC
This message is a reminder that Fedora 32 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora 32 on 2021-05-25.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
Fedora 'version' of '32'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 32 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 2 Debarshi Ray 2021-05-18 18:14:54 UTC
*** Bug 1882972 has been marked as a duplicate of this bug. ***

Comment 3 Debarshi Ray 2021-05-19 00:44:51 UTC
Things have changed a bit since GNOME Terminal 3.36.1.1 because of this commit:
  Revert "screen: Use clean env when creating new tab"

The crash should no longer happen unless the terminal is running a Toolbox shell inside it.

However, I think it's still worth fixing this crash for the Toolbox shell case.

As far as I can make out, the priv->exec_data of a TerminalScreen can only be NULL between creating the TerminalScreen instance with terminal_screen_new and executing a child in it with terminal_screen_exec. There are only two code paths where this happens - one when a new terminal is created from an existing terminal using the keyboard shortcuts or menus (ie., the GSimpleAction), and the other when creating a new terminal using the app menu or the gnome-terminal client (ie., over D-Bus).

As far as I can make out, a terminal created through the GSimpleAction executes a child in one single synchronous step. Therefore, it's not possible for it to be a parent TerminalScreen with a NULL priv->exec_data.

However, one that's created over D-Bus might have a NULL priv->exec_data between the successive D-Bus calls to o.g.Terminal.Factory:CreateInstance and o.g.Terminal.Receiver:Exec.

So, I think what happened here is that someone created a new terminal over D-Bus and then immediately used the GSimpleAction to create a second one from it between the two D-Bus calls.

Comment 4 Debarshi Ray 2021-05-19 00:55:06 UTC
We are only interested in the parent TerminalScreen's priv->exec_data when a Toolbox shell is running inside it. That implies that priv->exec_data can't be NULL because a child (ie., toolbox enter ...) has already been executed inside it. Otherwise, we wouldn't know that a Toolbox shell is running inside it.

Therefore, I think it would be sufficient to add a g_return_* assertion to express this assumption, and that the crash is actually fixed already.

Comment 5 Debarshi Ray 2021-05-31 15:17:50 UTC
I have updated the patches accordingly in Fedora >= 33. Since the crash is effectively fixed already, I held off from doing any builds except for Rawhide.


Note You need to log in before you can comment on or make changes to this bug.