nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

[BUG] Cannot close a driver that created a thread with open

Open SPRESENSE opened this issue 6 months ago • 1 comments

Description / Steps to reproduce the issue

This issue is caused by https://github.com/apache/nuttx/pull/16499.

The driver's open method creates a kernel thread. The kernel thread is deleted on close method.

static int xxx_open(FAR struct file *filep)
{
  kthread_create();

  return OK;
}

static int xxx_close(FAR struct file *filep)
{
  kthread_delete();

  return OK;
}

In fs_files.c updated by https://github.com/apache/nuttx/pull/16499, because f_refs is always greater than 2 for this driver, the driver's close method is never called.

int file_put(FAR struct file *filep)
{
  ...
  /* If refs is zero, the close() had called, closing it now. */

  if (atomic_fetch_sub(&filep->f_refs, 1) == 1)
    {
      ret = file_close(filep);
  ...

int fdlist_close(FAR struct fdlist *list, int fd)
{
  ...
  return file_put(filep);
}

On which OS does this issue occur?

[OS: Linux]

What is the version of your OS?

Ubuntu 22.04

NuttX Version

master

Issue Architecture

[Arch: all]

Issue Area

[Area: File System], [Area: Drivers]

Host information

No response

Verification

  • [x] I have verified before submitting the report.

SPRESENSE avatar Jun 20 '25 07:06 SPRESENSE

@Donny9 please take a look on this issue.

@SPRESENSE is that driver you are citing included into mainline?

ping @xiaoxiang781216 @raiden00pl

acassis avatar Jun 20 '25 17:06 acassis

@SPRESENSE Please let me know which driver code it is. You can take a look at the PR https://github.com/apache/nuttx/pull/16609 to see if it can resolve your issue. Currently, there are a few API usages that require attention:

  1. Obtain the corresponding filep through the file descriptor. After using file_get, you need to release the filep. Access both the filep and fd structures simultaneously by calling file_get2 to obtain them, enabling access to these two structures.

Image

Image

  1. Call file_allocate to request a new fd, but the corresponding filep will be empty. You need to initialize it using file_mq_vopen(example), and finally return the fd. Before returning, you must call file_put on the filep.

Image

  1. Call file_allocate_from_inode to request a new fd. Internally, it will first allocate a filep, initialize it through parameter alignment, and then bind the fd and filep.

Image

  1. Call fdlist_allocate to request a new fd and return the filep corresponding to the fd. This filep is not initialized. After initializing it with file_vopen, return the final fd. Before returning, you must call file_put on the filep.

Image

5.Call fdlist_dupfile to duplicate a specified filep and request a new fd to be returned.

Image

The summary before and after the modifications is as follows: Image

cc @acassis

Donny9 avatar Jun 24 '25 14:06 Donny9

@Donny9

@SPRESENSE Please let me know which driver code it is. You can take a look at the PR https://github.com/apache/nuttx/pull/16609 to see if it can resolve your issue.

For example, nuttx/drivers/modem/alt1250/alt1250.c nuttx/boards/arm/cxd56xx/drivers/sensors/cxd5610_gnss.c

  int fd;
  fd = open("/dev/alt1250", O_RDONLY); // alt1250_open is called
  close(fd);                           // alt1250_close is NOT called
  fd = open("/dev/alt1250", O_RDONLY); // ---> Failed to open file descriptor

I applied https://github.com/apache/nuttx/pull/16609, but it is not fixed.

SPRESENSE avatar Jun 24 '25 23:06 SPRESENSE

@xiaoxiang781216 @Donny9 @acassis Not yet fixed in https://github.com/apache/nuttx/pull/16609. It was closed by wrong link, please re-open.

SPRESENSE avatar Jun 26 '25 13:06 SPRESENSE

@SPRESENSE It is possible to create a simple test to verify this issue on SIM or QEMU?

acassis avatar Jun 26 '25 13:06 acassis

@SPRESENSE please try https://github.com/apache/nuttx/pull/16629, thanks.

Donny9 avatar Jun 26 '25 14:06 Donny9

@Donny9 I tried https://github.com/apache/nuttx/pull/16629, and it works well. Thanks.

SPRESENSE avatar Jun 26 '25 18:06 SPRESENSE