net/udp: Fix memory leak with UDP + write buffer is closed. Also update TODO and comments and refresh a configuration.

This commit is contained in:
Gregory Nutt 2018-01-23 11:54:03 -06:00 committed by Matt Thompson
parent 7f245e2720
commit 58b95f4629
6 changed files with 49 additions and 31 deletions

30
TODO
View File

@ -1,4 +1,4 @@
NuttX TODO List (Last updated January 21, 2018) NuttX TODO List (Last updated January 23, 2018)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This file summarizes known NuttX bugs, limitations, inconsistencies with This file summarizes known NuttX bugs, limitations, inconsistencies with
@ -19,7 +19,7 @@ nuttx/:
(8) Kernel/Protected Build (8) Kernel/Protected Build
(3) C++ Support (3) C++ Support
(6) Binary loaders (binfmt/) (6) Binary loaders (binfmt/)
(17) Network (net/, drivers/net) (16) Network (net/, drivers/net)
(4) USB (drivers/usbdev, drivers/usbhost) (4) USB (drivers/usbdev, drivers/usbhost)
(0) Other drivers (drivers/) (0) Other drivers (drivers/)
(12) Libraries (libc/, libm/) (12) Libraries (libc/, libm/)
@ -1075,23 +1075,19 @@ o Network (net/, drivers/net)
Title: POLL/SELECT ON TCP/UDP SOCKETS NEEDS READ-AHEAD Title: POLL/SELECT ON TCP/UDP SOCKETS NEEDS READ-AHEAD
Description: poll()/select() only works for availability of buffered TCP/UDP Description: poll()/select() only works for availability of buffered TCP/UDP
read data (when read-ahead is enabled). The way writing is read data (when read-ahead is enabled). The way writing is
handled in the network layer, all sockets must wait when send and handled in the network layer, either (1) If CONFIG_UDP/TCP_WRITE_BUFFERS=y
cannot be notified when they can send without waiting. then we never have to wait to send; otherwise, we always have
to wait to send. So it is impossible to notify the caller
when it can send without waiting.
An exception "never having to wait" is the case where we are
out of memory for use in write buffering. In that case, the
blocking send()/sendto() would have to wait for the memory
to become available.
Status: Open, probably will not be fixed. Status: Open, probably will not be fixed.
Priority: Medium... this does effect porting of applications that expect Priority: Medium... this does effect porting of applications that expect
different behavior from poll()/select() different behavior from poll()/select()
Title: SOCKETS DO NOT ALWAYS SUPPORT O_NONBLOCK
Description: sockets do not support all modes for O_NONBLOCK. Sockets
support nonblocking operations only (1) for TCP/IP non-
blocking read operations when read-ahead buffering is
enabled, (2) TCP/IP accept() operations when TCP/IP
connection backlog is enabled, (2) UDP/IP read() operations
when UDP read-ahead is enabled, and (3) non-blocking
operations on Unix domain sockets.
Status: Open
Priority: Low.
Title: INTERFACES TO LEAVE/JOIN IGMP MULTICAST GROUP Title: INTERFACES TO LEAVE/JOIN IGMP MULTICAST GROUP
Description: The interfaces used to leave/join IGMP multicast groups is non-standard. Description: The interfaces used to leave/join IGMP multicast groups is non-standard.
RFC3678 (IGMPv3) suggests ioctl() commands to do this (SIOCSIPMSFILTER) but RFC3678 (IGMPv3) suggests ioctl() commands to do this (SIOCSIPMSFILTER) but
@ -1124,8 +1120,8 @@ o Network (net/, drivers/net)
however. Others support the address filtering interfaces but however. Others support the address filtering interfaces but
have never been verifed: have never been verifed:
C5471, LM3S, ez80, DM0x90 NIC, PIC: Do not support address C5471, LM3S, ez80, DM0x90 NIC, PIC, LPC54: Do not support
filtering. address filtering.
Kinetis, LPC17xx, LPC43xx: Untested address filter support Kinetis, LPC17xx, LPC43xx: Untested address filter support
Status: Open Status: Open

View File

@ -630,12 +630,6 @@ Configurations
6. USB support is disabled by default. See the section above entitled, 6. USB support is disabled by default. See the section above entitled,
"USB Interface" "USB Interface"
STATUS. The first time I build the configuration, I get some undefined
external references. No idea why. Simply cleaning the apps/ directory
and rebuilding fixes the problem:
make apps_clean all
nsh: nsh:
This configuration directory provide the basic NuttShell (NSH). This configuration directory provide the basic NuttShell (NSH).

View File

@ -1,5 +1,4 @@
# CONFIG_NSH_ARGCAT is not set # CONFIG_NSH_ARGCAT is not set
# CONFIG_NSH_CMDOPT_DF_H is not set
# CONFIG_NSH_CMDOPT_HEXDUMP is not set # CONFIG_NSH_CMDOPT_HEXDUMP is not set
# CONFIG_NSH_CMDPARMS is not set # CONFIG_NSH_CMDPARMS is not set
CONFIG_ARCH_BOARD_VIEWTOOL_STM32F107=y CONFIG_ARCH_BOARD_VIEWTOOL_STM32F107=y
@ -23,9 +22,15 @@ CONFIG_NET_ARP_SEND=y
CONFIG_NET_BROADCAST=y CONFIG_NET_BROADCAST=y
CONFIG_NET_ETH_MTU=650 CONFIG_NET_ETH_MTU=650
CONFIG_NET_ETH_TCP_RECVWNDO=624 CONFIG_NET_ETH_TCP_RECVWNDO=624
CONFIG_NET_HOSTNAME="Viewtool-STM32F107"
CONFIG_NET_ICMP_SOCKET=y CONFIG_NET_ICMP_SOCKET=y
CONFIG_NET_ICMP=y CONFIG_NET_ICMP=y
CONFIG_NET_ICMPv6_NEIGHBOR=y
CONFIG_NET_ICMPv6_SOCKET=y
CONFIG_NET_ICMPv6=y
CONFIG_NET_IPv6=y
CONFIG_NET_MAX_LISTENPORTS=40 CONFIG_NET_MAX_LISTENPORTS=40
CONFIG_NET_ROUTE=y
CONFIG_NET_SOCKOPTS=y CONFIG_NET_SOCKOPTS=y
CONFIG_NET_TCP_CONNS=40 CONFIG_NET_TCP_CONNS=40
CONFIG_NET_TCP_WRITE_BUFFERS=y CONFIG_NET_TCP_WRITE_BUFFERS=y
@ -56,9 +61,9 @@ CONFIG_SCHED_HPWORK=y
CONFIG_SCHED_HPWORKPRIORITY=192 CONFIG_SCHED_HPWORKPRIORITY=192
CONFIG_SCHED_HPWORKSTACKSIZE=1024 CONFIG_SCHED_HPWORKSTACKSIZE=1024
CONFIG_SDCLONE_DISABLE=y CONFIG_SDCLONE_DISABLE=y
CONFIG_START_DAY=21 CONFIG_START_DAY=23
CONFIG_START_MONTH=9 CONFIG_START_MONTH=1
CONFIG_START_YEAR=2009 CONFIG_START_YEAR=2018
CONFIG_STM32_ETHMAC=y CONFIG_STM32_ETHMAC=y
CONFIG_STM32_JTAG_FULL_ENABLE=y CONFIG_STM32_JTAG_FULL_ENABLE=y
CONFIG_STM32_PHYSR_100MBPS=0x0000 CONFIG_STM32_PHYSR_100MBPS=0x0000
@ -70,6 +75,7 @@ CONFIG_STM32_PWR=y
CONFIG_STM32_RMII_EXTCLK=y CONFIG_STM32_RMII_EXTCLK=y
CONFIG_STM32_USART1=y CONFIG_STM32_USART1=y
CONFIG_SYSTEM_PING=y CONFIG_SYSTEM_PING=y
CONFIG_SYSTEM_PING6=y
CONFIG_TASK_NAME_SIZE=0 CONFIG_TASK_NAME_SIZE=0
CONFIG_USART1_SERIAL_CONSOLE=y CONFIG_USART1_SERIAL_CONSOLE=y
CONFIG_USER_ENTRYPOINT="nsh_main" CONFIG_USER_ENTRYPOINT="nsh_main"

View File

@ -345,7 +345,7 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
/* If we have a semi-permanent write buffer callback in place, then /* If we have a semi-permanent write buffer callback in place, then
* is needs to be be nullifed. * is needs to be be nullified.
* *
* Commit f1ef2c6cdeb032eaa1833cc534a63b50c5058270: * Commit f1ef2c6cdeb032eaa1833cc534a63b50c5058270:
* "When a socket is closed, it should make sure that any pending write * "When a socket is closed, it should make sure that any pending write
@ -357,6 +357,9 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
* data. However, to be able to actually send any new data, the send * data. However, to be able to actually send any new data, the send
* callback must be left. The callback should be freed later when the * callback must be left. The callback should be freed later when the
* socket is actually destroyed." * socket is actually destroyed."
*
* REVISIT: Where and how exactly is s_sndcb ever freed? Is there a
* memory leak here?
*/ */
psock->s_sndcb = NULL; psock->s_sndcb = NULL;
@ -548,7 +551,18 @@ int inet_close(FAR struct socket *psock)
if (conn->crefs <= 1) if (conn->crefs <= 1)
{ {
/* Yes... free the connection structure */ /* Yes... */
#ifdef CONFIG_NET_UDP_WRITE_BUFFERS
/* Free any semi-permanent write buffer callback in place. */
if (psock->s_sndcb != NULL)
{
udp_callback_free(conn->dev, conn, psock->s_sndcb);
psock->s_sndcb = NULL;
}
#endif
/* And free the connection structure */
conn->crefs = 0; conn->crefs = 0;
udp_free(psock->s_conn); udp_free(psock->s_conn);

View File

@ -112,7 +112,11 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev,
eventset |= POLLIN & info->fds->events; eventset |= POLLIN & info->fds->events;
} }
/* A poll is a sign that we are free to send data. */ /* A poll is a sign that we are free to send data.
* REVISIT: This is bogus: If CONFIG_TCP_WRITE_BUFFERS=y then
* we never have to wait to send; otherwise, we always have to
* wait to send. Receiving a poll is irrelevant.
*/
if ((flags & TCP_POLL) != 0) if ((flags & TCP_POLL) != 0)
{ {

View File

@ -111,7 +111,11 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev,
eventset |= (POLLIN & info->fds->events); eventset |= (POLLIN & info->fds->events);
} }
/* poll is a sign that we are free to send data. */ /* A poll is a sign that we are free to send data.
* REVISIT: This is bogus: If CONFIG_UDP_WRITE_BUFFERS=y then
* we never have to wait to send; otherwise, we always have to
* wait to send. Receiving a poll is irrelevant.
*/
if ((flags & UDP_POLL) != 0) if ((flags & UDP_POLL) != 0)
{ {