FD leak in the progress_bar_thread (progress_thread.c)

[Michael Glembotzki]

Hi all,

We are using an application that connects to the progress socket via IPC.
There was an error in our application, causing it to restart repeatedly. This
resulted in more and more connfd's [1] being added to the queue. I noticed that
the socketfd's are never closed after an external error occurs. Presumably, this
is because no data is ever read from the socket, and no write operation takes
place. This quickly leads to a FD leak [2]. A close(connfd); is also missing
here [3]. However, I don't think this explains the error.

[TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too many open files
[TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too many open files
[TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too many open files
[TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too many open files
[...]

[1] https://github.com/sbabic/swupdate/blob/master/core/progress_thread.c#L347
[2] https://github.com/sbabic/swupdate/blob/master/core/progress_thread.c#L322
[3] https://github.com/sbabic/swupdate/blob/master/core/progress_thread.c#L335-L336

What should we do about this?

a) Nothing, because it's an external problem?
b) Add a Max Limit for the queue?
c) Add a dummy read after the accept() method?
Or do you have any other ideas?

Best regards,
Michael

[Stefano Babic]

Hi Michael,

On 11/21/25 10:43, Michael Glembotzki wrote:
> Hi all,
>
> We are using an application that connects to the progress socket via IPC.
> There was an error in our application, causing it to restart repeatedly.
> This
> resulted in more and more connfd's [1] being added to the queue.

Sure, and max open files is reached. It is known.

> I
> noticed that
> the socketfd's are never closed after an external error occurs.

Right - the housekeeping for the connection is done only when the
progress have to send something, and then connections are closed. This
is done in send_progress_msg()

In the situation you describe, it is not possible to know if there is a
use case with a lot of clients or if an application is gone wild.

> Presumably, this
> is because no data is ever read from the socket,

or sent

> and no write operation
> takes
> place.

Right.

> This quickly leads to a FD leak [2].

It is not a leak - the fd is stored in the connection list, and from
SWUpdate's point of view it is ok.

> A close(connfd); is also missing
> here [3]. However, I don't think this explains the error.
>
> [TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too
> many open files
> [TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too
> many open files
> [TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too
> many open files
> [TRACE] : SWUPDATE running : [progress_bar_thread] : Accept returns: Too
> many open files
> [...]
>
> [1] https://github.com/sbabic/swupdate/blob/master/core/
> progress_thread.c#L347
> [2] https://github.com/sbabic/swupdate/blob/master/core/
> progress_thread.c#L322
> [3] https://github.com/sbabic/swupdate/blob/master/core/
> progress_thread.c#L335-L336
>
> What should we do about this?
>
> a) Nothing, because it's an external problem?

This was already reported in the past and this is my position. We cannot
fix in SWUpdate bugs in external programs.

> b) Add a Max Limit for the queue?

It is then difficult to say which is the limit because there are use
case that we do not know.

> c) Add a dummy read after the accept() method?

No.

You cannot even know if your application crashes after the connection or
after some time, so this does not resolve anything.

> Or do you have any other ideas?

As far as Iknow when I have investigated, there is no socket option to
check the status of a connection, and SWUpdate knows that a connection
is broken when it tries to write something.

Best regards,
Stefano

[Michael Glembotzki]

Hi Stefano,

I am okay with the decision! Nevertheless are you adding the missing close() in [3]?

 

> b) Add a Max Limit for the queue?

It is then difficult to say which is the limit because there are use
case that we do not know.

> c) Add a dummy read after the accept() method?

No.

You cannot even know if your application crashes after the connection or
after some time, so this does not resolve anything.

> Or do you have any other ideas?

As far as Iknow when I have investigated, there is no socket option to
check the status of a connection, and SWUpdate knows that a connection
is broken when it tries to write something.

Best regards,
Stefano

Best regards,
Michael 

[Stefano Babic]

Hi Michael,

Sure, I send the patch.

Stefano

>
> > b) Add a Max Limit for the queue?
>
> It is then difficult to say which is the limit because there are use
> case that we do not know.
>
> > c) Add a dummy read after the accept() method?
>
> No.
>
> You cannot even know if your application crashes after the
> connection or
> after some time, so this does not resolve anything.
>
> > Or do you have any other ideas?
>
> As far as Iknow when I have investigated, there is no socket option to
> check the status of a connection, and SWUpdate knows that a connection
> is broken when it tries to write something.
>
> Best regards,
> Stefano
>
>
> Best regards,
> Michael
>

> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/b39f109c-b7c8-4a28-9b7e-1dac61c4472en%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/b39f109c-
> b7c8-4a28-9b7e-1dac61c4472en%40googlegroups.com?
> utm_medium=email&utm_source=footer>.