anchao commented on code in PR #21439:
URL: https://github.com/apache/nuttx/pull/21439#discussion_r4476198814
##########
fs/inode/fs_files.c:
##########
@@ -175,6 +180,15 @@ static int fdlist_extend(FAR struct fdlist *list, size_t
row)
return OK;
}
+ /* Release the buffers we allocated for any rows that the current list
+ * already supplies (its pointers will be copied in below).
+ */
+
+ for (j = orig_rows; j < i && j < list->fl_rows; j++)
+ {
+ fs_heap_free(fds[j]);
Review Comment:
Thanks both for the careful review — you're right about the problem.
@casaroli the concern is valid: the row-by-row merge I added called
fs_heap_free() while fl_lock was still held (and interrupts off), and
fs_heap_free() → mm_lock() → nxmutex_lock() can sleep, which
spin_lock_irqsave() forbids.
@jingfei195887 I went with your goto retry approach. On detecting
list->fl_rows != orig_rows (another thread grew the list while we were
allocating), I now drop the lock, release the temporary buffers, and jump back
to retry: — where orig_rows is re-read. If the winning thread already extended
the list to >= row, the function returns 0 from the top; otherwise it simply
re-allocates only the rows that are still missing. This keeps fs_heap_free()
strictly outside the spinlock while preserving the original leak fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
