The stupidity and arrogance of GNOME developers

About a year ago I wrote a blog post explaining how the horrible development practices of people in the GNOME project lead to a bug in vte that was annoying me, even though I don’t use GNOME: GNOME’s horrid coding practices.

The post is highly technical, but if you manage to go through it, it’s clear they made a mistake, and they refused to acknowledge their mistake, even though several users complained about the bug for years (not just me).

But the real problem is not that they refused to fix the bug, it’s that I provided them with a perfectly valid fix, and they refused to even look into the merit of it, only because of their ego.

Today there’s an update: they came up with a fix of their own. However, the irony is that their fix is an obfuscated and poorer version of my own fix.

What’s the problem then? Many are going to presume this is some kind of “I told you so” retribution to elevate my ego. No, the problem is that my fix was available three years ago. All they managed to accomplish by ignoring and censoring me, is hurt their own users.

They could have applied my fix a long time ago.

Now, before going into the complex details, I think it’s useful to clarify that the bug was introduced because they were not sending a signal of an event, when the event happens. But even though the issue is complex, the fix is extremely simple: send the signal of the event right after the event happens. That’s literally it. That’s what my fix did, and that’s what they ultimately ended up doing.

For three years they argued that delaying the signal was good, only to come up with a patch that doesn’t delay the signal, which is exactly what I argued should be the case in the first place.

If they made a mistake and realized it several years later, that would be understandably human, but that’s not what’s happening. I’m arguing they are stupid not because they made mistakes, but because they still haven’t realized that’s what they did. So it’s very likely that they will make the same mistakes again.

This is a summary of the mistakes they did when dealing with this regression:

• Did not detect the introduction of a regression
• Refused to revert the change that introduced the regression
• Worked around the regression
• Refused a perfectly valid fix
• Refused to consider the merit of the valid fix
• Refused to consider other alternatives to mitigate the regression
• Locked and censored the issue in their tracker
• Refused to acknowledge there was a bug despite multiple reports
• Ultimately applied the valid fix anyway
• Never acknowledged the valid fix was already available

The issue

To understand the issue we don’t need to understand everything there is about virtual terminals, all we need to understand are two signals: child-exited and end-of-stream.

child-exited

When you run a terminal (like gnome-terminal) there is always a process that is driving everything, for example bash. This is the “child” process.

When the child exits, typically you would want the terminal to exit as well (although you can configure it to remain open). Ultimately it’s up to the end user to decide what to do, but the job of the terminal library — vte in this case — is to provide the relevant information to the terminal — like gnome-terminal — in order to do what the end user wants.

This is a convoluted way of saying that when bash exits, that needs to be communicated to gnome-terminal so it can exit right away — as 99.9% of the users would want.

end-of-stream

In most cases the stream ends when the child exits. By “stream” we can think of the standard output. So when bash exits, there would be no more output — because the program cannot execute any more commands: it’s done. Once the program is done, all the file descriptors are cosed — including stdout.

bash -c 'echo "bye dad"'

But this isn’t always the case. A process can always launch subprocesses. And each one of those subprocesses would inherit the standard streams. So all the children of bash are going to inherit the standard output of bash.

bash -c '(sleep 10; echo "bye grandpa") & exit'

In the above example, bash exits immediately, but it spawned a child, and that child is going to print “bye grandpa” after 10 seconds. This means the stdout file descriptor cannot be closed yet (not until all the children of bash exit).

So the end-of-stream signal is not going to be sent when the child exits, but when the grandchild exits, because the stdout file descriptor is going to remain open when bash exits.

Give it to me straight

What does this mean in practical terms?

In practical terms it means that when bash exits, the stdout file descriptor might not be closed yet. In other words: the end-of-stream signal might come after the child-exited signal.

The bug that was introduced in 2019, is that if the end-of-stream signal hadn’t come yet, then the child-exited signal was delayed. And this delay could be infinite.

So how on Earth would a terminal — like gnome-terminal — know when to exit? It didn’t. And that was the regression.

To exit immediately, the terminal needed to receive the child-exited signal immediately. And after the commit 7888602c (lib: Rework child exit and EOF handling) that wasn’t happening anymore: it was delayed ad infinitum.

First the delay was infinite, then it was capped to 5 seconds, and eventually it was capped to 2 seconds. But the delay was there, and that was the problem.

The fix

If the user wants the terminal to exit immediately, then the child-exited signal needs to be sent immediately. It’s as simple as that.

Stupidly obvious

The complexity of the explanation of the issue above might have thrown some people off, so I want to make sure people understand the fix.

The fix is to send the signal of the event, when the event happens. So the fix is to send the child-exited signal, when the child exits.

I don’t know how the fix could be any simpler.

I don’t pay you to think

If you tell someone — like your assistant — “please monitor NVDA stock price and let me know when it’s bellow $850”. Then if you learn that the stock price is at $825 you would be understandably upset with this guy, but don’t worry, you assistant has an explanation.

He might calmly explain to you “I know your objective is to make money, so I surmised the best way to make money is to buy the stock when the price is $800, the price is only $825, so it’s not yet time to buy”.

But you didn’t tell him “notify me when you think it’s time to buy”, did you? You never delegated that decision to him.

This bug is very similar. The terminal is telling vte: “please notify me when the child exits”, and vte comes back and says “it’s still not time to close the terminal”. But that is not what the terminal asked.

You see, it’s not just that GNOME developers think they know better than their users, it’s also that the developers of GNOME libraries think they know better than the users of those libraries: the developers of vte think they know better than the developers of xfce4-terminal (and countless others).

They claimed it was desirable to send the child-exited signal when they believed the terminal should be closed, instead of when the child had exited.

Rogue library

I did explain to them that the child-exited signal should be sent when the child exits.

One has to do pretty advanced mental gymnastics to argue that a child-exited signal is not useful to issue at the moment a child exits.

Felipe Contreras

They argued that it was they the ones that should decide when the terminal can be closed:

To summarize the experience we have gained so far, both in the original bug, and then to the less-than-ideal “fix” to that bug (which broke something else), the desired behavior is the combination of all of these, at the same time:

1. Process all output from the immediate child.
2. After that, quit (or restart, or whatever) ASAP.
3. No need to process all the output from grandchildren, their descendants, or other processes that somehow got access to the tty line.

Egmont Koblinger

Egmont clearly doesn’t understand how standard streams on forks work, because there’s no way to distinguish output from the child and output from the grandchildren. But even if this was somehow possible, it should be up to the terminal to decide when to exit: if the terminal wants to exit before all the output from the child has been processed, it should be able to do so.

Ultimately is I — the end user — the one that should decide, not the terminal, and certain not the terminal library. The job of the library is to help achieve what the end user wants, not make decisions for the end user.

If the end user wants the terminal to close immediately and not display any output, vte should just shut up and do that.

In the end his solution did not achieve anything my solution didn’t either, as we’ll soon see.

Proof

At this point you might be thinking that there’s no way GNOME developers could possibly be so oblivious about their own code that an outsider like me could pinpoint to such glaring mistakes.

Well, let’s see.

This is the core of the problem before the latest fix was applied, in the Terminal::child_watch_done method:

/* If we still have a PTY, or data to process, defer emitting the signals
 * until we have EOF on the PTY, so that we can process all pending data.
 */
if (pty() || !m_incoming_queue.empty()) {
        m_child_exit_status = status;
        m_child_exited_after_eos_pending = true;

        m_child_exited_eos_wait_timer.schedule_seconds(2); // FIXME: better value?
} else {
        m_child_exited_after_eos_pending = false;

        if (widget())
                widget()->emit_child_exited(status);
}

You don’t need to understand what all that is doing, all you need to know is that my fix changed that to:

if (widget())
        widget()->emit_child_exited(status);

What did their fix do?

/* If we still have a PTY, or data to process, defer emitting the signals
 * until we have EOF on the PTY, so that we can process all pending data.
 */
if (pty()) {
        /* Read and process about 64k synchronously, up to EOF or EAGAIN
         * or other error, to make sure we consume the child's output.
         * See https://gitlab.gnome.org/GNOME/vte/-/issues/2627 */
        pty_io_read(pty()->fd(), G_IO_IN, 65536);
        if (!m_incoming_queue.empty()) {
                process_incoming();
        }

        /* Stop processing data. Optional. Keeping processing data from grandchildren and
         * other writers would also be a reasonable choice. It makes a difference if the
         * terminal is held open after the child exits. */
        unset_pty();
}

if (widget())
        widget()->emit_child_exited(status);

There is more code, but at the end they call the Widget::emit_child_exited method, which is exactly the same thing I did.

So now they are sending the child-exited signal, when the child exits. Brilliant idea!

You may be thinking that perhaps the rationale as to why the signal can be sent at that moment comes from the extra code above, I’ll get into that. But the important thing is that after it’s done, the signal is sent. Period.

The difference

Perhaps the chunk of code they added changes everything?

Here’s a video showing what happens with their fix:

And here’s the video showing what happens with my fix:

Can you spot the difference? I doubt that you can, because there is no difference.

So reading that 64 k of data was not necessary, they could have simply applied my patch. Now, I could go into all the details to explain why there isn’t any difference, but it’s not necessary because this isn’t even the use case that matters. The relevant use case — as we’ll see below — is when the terminal is held open, so there’s even more time to receive the data.

They delayed the fix for absolutely no gain.

Defence

Maybe I’m being unfair and they probably had a good reason. Let’s explore their rationale.

The developers created yet another issue ticket to discuss their “proper” fix (Exit delay in presence of grandchildren processes). There Egmont spent 4102 words just to begin to justify his proposal. But in the end his proposal is simple:

Approaching from two different directions, the traffic-based safety cap seems to be by far the best bet for me. Plus, the synchronous approach looks safe enough and simpler than the mainloop-based one. Therefore this is what my patch does. When we notice that the child has quit, we synchronously read up to EOF, EAGAIN, any other error condition, or about 64k of data, whichever comes first, and process that data before emitting the ever so slightly deferred child-exited signal.

Egmont Koblinger

This explains the chunk of code we saw before: it’s trying to read 64 k of data synchronously before sending the child-exited signal, so now it’s only “ever so slightly deferred”.

I already proved that chunk makes no difference, but let’s assume that they are right and it did make a difference. Why would reading 64 k data be important?

The origin story

The regression traces back to trying to fix issue 2366 (End of cmd output discarded with “Hold the terminal open”). But the thing to keep in mind is that this is only relevant when you hold the terminal open.

Say you launch a terminal in order to run a single command:

xfce4-terminal -e fortune

Naturally you would like to see the output of that command. But if the terminal exits when the command finishes, then you won’t see anything. In order to see the output after the child has exited, you need to hold the terminal open:

xfce4-terminal --hold -e fortune

The issue reported in 2366 is that not all the output was displayed, if two things happened:

• The output is more than 8 k of data
• The terminal is held open

So it was very marginal issue which probably affected less than 1% of users (more like 0.01%).

Worse than the disease

The “fix” that caused more problems than it solved (lib: Rework child exit and EOF handling) did dozens of changes in a single commit, which made it virtually impossible to realize what fixed the problem. But after I spent an inordinate amount of time splitting the more than 600 lines of code changed into 25 logically atomic commits, it became clear what fixed the issue:

--- a/src/vte.cc
+++ b/src/vte.cc
@@ -3265,9 +3265,6 @@ Terminal::child_watch_done(pid_t pid,

         m_pty_pid = -1;

-        /* Close out the PTY. */
-        unset_pty();
-
         /* Tell observers what's happened. */
         if (m_real_widget)
                 m_real_widget->emit_child_exited(status);

It would take time to explain what a PTY is, but you don’t need to know that: all you need to know is that it’s the main channel of communication. So if the PTY is closed, the link between the terminal and the child process is severed. This means the streams are forcibly closed, and therefore any output coming from the child process after this point would be lost.

If the PTY is not closed, then the terminal is able to receive output from the child after the child-exited signal is raised.

Essentially the fix decouples the closing of the streams from the child-exited signal.

So what did the following 24 changes do? Nothing useful. In fact, the last 2 changes rejoined the two signals. Before: the streams were being closed when child-exited was sent, and after: the child-exited signal was sent when the streams were closed.

So one issue was replaced by another issue, except that the new issue was much worse.

The myth

The misconception that vte developers were never able to overcome is that all that was needed to avoid the issue reported in 2366 was to not close the PTY, and thus decoupling the two events: closing of the streams from the child exiting.

Because they are unable to put themselves in the shoes of the terminal, they don’t understand that they sending the signal child-exited doesn’t automatically result in the terminal being closed. The terminal has autonomy, and it can decide to ignore the child-exited signal. I even wrote a simple vte client terminal precisely to test all the uses cases, and I used it to prove that when the terminal is held open, my fix works perfectly fine, which I explained in the bug report. In the issue 2366 it’s explicitly stated that the problem is relevant only when the “hold the terminal open” option is selected.

When “hold the terminal open” is selected, child-exited does not terminate the terminal.

The terminal is able to keep displaying output from the child, even after child-exited was sent, as long as the file descriptors are not closed.

So they didn’t need to delay child-exited: all they needed to do is not close the PTY.

They never actually understood why the patch fixed the issue: they wrongly attributed it to delaying the signal.

The 64 k chunk

We come back to their fix:

/* If we still have a PTY, or data to process, defer emitting the signals
 * until we have EOF on the PTY, so that we can process all pending data.
 */
if (pty()) {
        /* Read and process about 64k synchronously, up to EOF or EAGAIN
         * or other error, to make sure we consume the child's output.
         * See https://gitlab.gnome.org/GNOME/vte/-/issues/2627 */
        pty_io_read(pty()->fd(), G_IO_IN, 65536);
        if (!m_incoming_queue.empty()) {
                process_incoming();
        }

        /* Stop processing data. Optional. Keeping processing data from grandchildren and
         * other writers would also be a reasonable choice. It makes a difference if the
         * terminal is held open after the child exits. */
        unset_pty();
}

if (widget())
        widget()->emit_child_exited(status);

There’s always going to be a PTY, so you can ignore the if (pty()) condition, then the logic consists of 3 steps:

1. Read a chunk of 64 k data
2. Close the PTY
3. Emit child-exited

Any data that is going to be read has to be before closing the PTY. Normally sending child-exited is going to close the PTY (in fact terminate the entire program), but not when the “hold the terminal open” option is selected, which is what started it all. So data can be read after child-exited in this case.

So we don’t need to do step 2 (which in fact has a comment saying it’s optional and “it makes a difference if the terminal is held open after the child exits”, and it is held open, therefore it makes a difference), and if we don’t do step 2, then we don’t need to do step 1 either: because the data can be read after child-exited is emitted.

If you skip step 2 and step 1 (which are unnecessary), then the code is exactly the same as my fix.

Verdict

They have no defence. Their code makes no difference.

The journey

In November 2019 this is what the relevant code in Terminal::child_watch_done looked like before introducing the regression.

/* Close out the PTY. */
unset_pty();

/* Tell observers what's happened. */
if (widget())
        widget()->emit_child_exited(status);

At this point the bug reported in 2366 was present: displaying more than 8 k data right as the child exits didn’t work.

However, the fix was simply to remove the unset_pty() line and don’t close the PTY. Instead, they chose to add several layers of complexity:

/* If we still have a PTY, or data to process, defer emitting the signals
 * until we have EOF on the PTY, so that we can process all pending data.
 */
if (pty() || !m_incoming_queue.empty()) {
        m_child_exit_status = status;
        m_child_exited_after_eos_pending = true;

        m_child_exited_eos_wait_timer.schedule_seconds(2); // FIXME: better value?
} else {
        m_child_exited_after_eos_pending = false;

        if (widget())
                widget()->emit_child_exited(status);
}

So while trying to fix issue 2366 which was very marginal — it happened only when the data exceeded 8 k and the terminal was configured to remain open and affected less than 0.1% of users — they introduced a regression that affected 100% of users.

The thing to note is that m_child_exited_after_eos_pending is going to eventually trigger the child-exited signal by forcing an end-of-stream signal, but that could take up to 2 seconds.

They argued for years that this was necessary in order for the use case described in issue 2366 to work. But that wasn’t true: they never understood that what fixed the issue was removing the unset_pty() call.

Finally in August 2023 — around 4 years later — they finally fix the regression with:

/* If we still have a PTY, or data to process, defer emitting the signals
 * until we have EOF on the PTY, so that we can process all pending data.
 */
if (pty()) {
        /* Read and process about 64k synchronously, up to EOF or EAGAIN
         * or other error, to make sure we consume the child's output.
         * See https://gitlab.gnome.org/GNOME/vte/-/issues/2627 */
        pty_io_read(pty()->fd(), G_IO_IN, 65536);
        if (!m_incoming_queue.empty()) {
                process_incoming();
        }

        /* Stop processing data. Optional. Keeping processing data from grandchildren and
         * other writers would also be a reasonable choice. It makes a difference if the
         * terminal is held open after the child exits. */
        unset_pty();
}

if (widget())
        widget()->emit_child_exited(status);

But they reintroduce the unset_pty(), when the comment itself says “optional“. If you remove that the code above that reads 64 k of data unecessarily, we end up with:

if (widget())
        widget()->emit_child_exited(status);

Which was precisely my fix.

So let’s get this straight, we went from this code in 2019:

/* Close out the PTY. */
unset_pty();

/* Tell observers what's happened. */
if (widget())
        widget()->emit_child_exited(status);

To this code in 2023:

/* A chunk of code that makes no difference. */

if (widget())
        widget()->emit_child_exited(status);

Which does exactly the same as my fix that they rejected for no good reason.

All these years I argued that the two signals should be decoupled, and eventually they did decouple them. Forget about all the countless hours I spent analyzing the code, trying different approaches, and arguing with these obtuse people; what did we (the open source community) gain by this ordeal? Absolutely nothing. The code they eventually added and wasn’t present in my fix makes no difference whatsoever.

But wait…

When I said it makes no difference, that’s a lie, it’s actually worse.

If you run the following command you would expect to see the “bye grandpa” output after 10 seconds.

xfce4-terminal --hold -e "bash -i -c '(sleep 10; echo bye grandpa) & exit'"

But it does not happen… because they decided to close the PTY.

In my fix that command works fine.

I’ve opened a new issue ticket for that, and I’ve sent a patch with the fix which is simply removing the unnecessary call to unset_pty(). If they apply my fix, then it becomes even more obvious that reading those 64 k is completely unnecessary. But I would not be holding my breath.

Lack of attention

In the process of understanding this issue I reverted a lot of changes that I thought might be unnecessary, and while doing so I found that they indeed were unnecessary. I did so very carefully to make sure I wasn’t missing anything, and especially because all this code was new to me.

That’s how I realized vte developers were not careful at all: they often removed the calls to methods without removing the methods.

Take for example this chunk:

@@ -10283,19 +10287,12 @@ Terminal::emit_pending_signals()
                 m_bell_pending = false;
         }

-        auto const eos = m_eos_pending;
         if (m_eos_pending) {
                 queue_eof();
                 m_eos_pending = false;

                 unset_pty();
         }
-
-        if (m_child_exited_after_eos_pending && eos) {
-                /* The signal handler could destroy the terminal, so send the signal on idle */
-                queue_child_exited();
-                m_child_exited_after_eos_pending = false;
-        }
 }

 void

The call to queue_child_exited() is removed, but after that there isn’t any call to that method, so why aren’t they removing the method? Because they aren’t paying attention to their own code.

Terminal::queue_child_exited queues a call to emit_child_exited_idle_cb (which probably could have been called directly anyway), and that calls Terminal::emit_child_exited (which probably didn’t need to be a separate function), and that finally calls:

if (widget())
        widget()->emit_child_exited(status);

Which is now called right away, so obviously it’s not needed anymore.

All these methods can be removed, and therefore m_child_exit_status can be removed as well.

The same happened in the regression patch, where they removed the definition of Terminal::pty_channel_eof, but not the declaration.

My version of the fix did correctly remove all the code that was not used any more. That’s why I say their version is poorer.

I’ve sent a patch to fix all their oversights, but I think we all can guess how that is likely going to go.

Really?

After it was all said and done, Egmont found an interesting comment in the luit command:

-x Exit as soon as the child dies. This may cause luit to lose data at the end of the child’s output.

luit(1)

…

They suffer from the same dilemma, and delegate the decision to the user.

Egmont Koblinger

No shirt Sherlock. The end user should decide how the software behaves? Who would have thought of that?

But you are wrong, they are not “delegating” the decision to the end user, it was always the user’s decision in the first place.

First principle of software

I am convinced most developers do not even understand what software is, and it’s very clear that includes GNOME developers.

The biggest thing any program can do is not the technical details of the program itself: it’s how useful the program is to users.

So any time a program – like the kernel or any other project – breaks the user experience, to me that is the absolute worst failure that a software project can make. It’s the complete no-no to ever break userspace – or for other projects – to ever break features that your users depend on.

Because no project is more important than the users of the project.

Linus Torvalds

The whole point of software is to be useful to users. That’s it.

Your job Egmont — the developer, is to make things easier for me — the end user. Not harder.

It’s the end user that should decide how the software behaves. If you are purposely making the software less useful to the end users, you are failing as a software developer. Period.

Their sins

Once you understand what software is, the list of catastrophic mistakes they did is obvious:

1. They didn’t revert the patch that introduced the regression. This is what projects that do understand the purpose of software do, I provided an example from the Linux project in my other blog post.
2. They didn’t revert the behavior that caused the regression. Even if they didn’t want to revert the whole patch — which is understandable because it’s a big patch with 600 lines of code (an awful practice to begin with) — they could have changed one line of code to restore the previous behavior.
3. They didn’t consider ways to mitigate the regression. Even if they didn’t want to go back to a state where issue 2366 was still present, they could have considered a middle ground, at least until a proper fix is found.
4. They didn’t allow the frustration of the end users to be heard. Even if they considered their solution to be correct, locking the issue and threatening people who criticized this approach with permanent bans is not what a project that cares about being useful to its users should do.
5. They didn’t analyze any of the proposed fixes from outsiders. Even if preserving the fix for 2366 was paramount, they could have analyzed the fixes that claimed to not break 2366, as my fix did, but they didn’t.

Even if you give them the benefit of the doubt and consider that the code they finally introduced in 2023: read 64 k of data was somehow necessary (even though it doesn’t make any difference), they shouldn’t have enabled the new behavior until they had that “proper” code.

If that meant that issue 2366 would not have been properly fixed until 2023, then so be it.

It is better to make 0.01% of users suffer for 4 more years, than introduce a regression and make 100% of users suffer for 4 years. Who could argue otherwise?

But that’s not even the case, because my fix made everything work correctly, and it was available years ago.

Conclusion

These people made their users suffer unnecessarily for 4 years, for no reason.

When I say they are “stupid”, it’s not out of spite, it’s an objective fact. Who argues that the child-exited signal should not be sent when the child exits, only to eventually write a patch that does precisely that?

And when I say they are “arrogant” it’s not because they rejected my solution, it’s because they couldn’t even entertain the possibility that some outsider could be more correct than them, and that’s why they didn’t even look at my patch.

This is just one issue that we can leave behind in the past, but the real problem is their bad development practices and their shitty attitude. That is not constrained to the vte project, but the whole of GNOME and I have similar horror stories in other components to prove it.

This isn’t about me, or my fix, this is about documenting the way GNOME developers operate in the details, and I think any objective observer would reach the conclusion that it’s simply not great.

Postscript

Because this isn’t my first rodeo, I can already foresee the criticism of this article: people are going to focus on my persona and my tone, not on what matters: GNOME‘s shitty practices.

Let’s make one thing clear: even if Satan was the one providing a patch, if the patch is valid and fixes a real issue, you apply the patch, period. Why? Because that’s what is best for the users.

I did nothing but try to fix their awful code. The issue is not me. I don’t even care what they do, I already moved on to a non-vte terminal: kitty (which does have an option to exit immediately: close_on_child_death).

People should ignore me and focus on how the GNOME project is going to handle similar situations in the future, even though I know many are going to do precisely the opposite.

Bonus

Some people don’t believe that my fix is valid, so I created a video showing step by step how it does work perfectly fine, contrary to what Egmont claimed.