Merge pull request #9248 from kjbracey-arm/socket_pending_atomic

0xc0170 · web-flow · commit 67dd4496bfa9 · 2019-01-21T13:35:47.000+01:00
InternetSocket: use atomics, not volatile
diff --git a/UNITTESTS/features/netsocket/DTLSSocket/unittest.cmake b/UNITTESTS/features/netsocket/DTLSSocket/unittest.cmake
@@ -22,6 +22,7 @@ set(unittest-test-sources
   features/netsocket/DTLSSocket/test_DTLSSocket.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   ../features/nanostack/coap-service/test/coap-service/unittest/stub/mbedtls_stub.c
   stubs/EventQueue_stub.cpp
diff --git a/UNITTESTS/features/netsocket/DTLSSocketWrapper/unittest.cmake b/UNITTESTS/features/netsocket/DTLSSocketWrapper/unittest.cmake
@@ -21,6 +21,7 @@ set(unittest-test-sources
   features/netsocket/DTLSSocketWrapper/test_DTLSSocketWrapper.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   ../features/nanostack/coap-service/test/coap-service/unittest/stub/mbedtls_stub.c
   stubs/EventQueue_stub.cpp
diff --git a/UNITTESTS/features/netsocket/InternetSocket/test_InternetSocket.cpp b/UNITTESTS/features/netsocket/InternetSocket/test_InternetSocket.cpp
@@ -79,14 +79,6 @@ class stubInternetSocket : public InternetSocket {
     {
         _writers--;
     }
-    void add_pending(void)
-    {
-        _pending++;
-    }
-    void rem_pending(void)
-    {
-        _pending--;
-    }
 
 protected:
     virtual nsapi_protocol_t get_proto()
diff --git a/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake b/UNITTESTS/features/netsocket/InternetSocket/unittest.cmake
@@ -18,6 +18,7 @@ set(unittest-test-sources
   features/netsocket/InternetSocket/test_InternetSocket.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   stubs/EventQueue_stub.cpp
   stubs/mbed_shared_queues_stub.cpp
diff --git a/UNITTESTS/features/netsocket/TCPServer/unittest.cmake b/UNITTESTS/features/netsocket/TCPServer/unittest.cmake
@@ -22,6 +22,7 @@ set(unittest-sources
 set(unittest-test-sources
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   stubs/EventQueue_stub.cpp
   stubs/mbed_shared_queues_stub.cpp
diff --git a/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake b/UNITTESTS/features/netsocket/TCPSocket/unittest.cmake
@@ -19,6 +19,7 @@ set(unittest-test-sources
   features/netsocket/TCPSocket/test_TCPSocket.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   stubs/EventQueue_stub.cpp
   stubs/mbed_shared_queues_stub.cpp
diff --git a/UNITTESTS/features/netsocket/TLSSocket/unittest.cmake b/UNITTESTS/features/netsocket/TLSSocket/unittest.cmake
@@ -21,6 +21,7 @@ set(unittest-test-sources
   features/netsocket/TLSSocket/test_TLSSocket.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   ../features/nanostack/coap-service/test/coap-service/unittest/stub/mbedtls_stub.c
   stubs/EventQueue_stub.cpp
diff --git a/UNITTESTS/features/netsocket/TLSSocketWrapper/unittest.cmake b/UNITTESTS/features/netsocket/TLSSocketWrapper/unittest.cmake
@@ -20,6 +20,7 @@ set(unittest-test-sources
   features/netsocket/TLSSocketWrapper/test_TLSSocketWrapper.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   ../features/nanostack/coap-service/test/coap-service/unittest/stub/mbedtls_stub.c
   stubs/EventQueue_stub.cpp
diff --git a/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake b/UNITTESTS/features/netsocket/UDPSocket/unittest.cmake
@@ -19,6 +19,7 @@ set(unittest-test-sources
   features/netsocket/UDPSocket/test_UDPSocket.cpp
   stubs/Mutex_stub.cpp
   stubs/mbed_assert_stub.c
+  stubs/mbed_critical_stub.c
   stubs/equeue_stub.c
   stubs/EventQueue_stub.cpp
   stubs/mbed_shared_queues_stub.cpp
diff --git a/features/netsocket/InternetSocket.cpp b/features/netsocket/InternetSocket.cpp
@@ -21,9 +21,10 @@ using namespace mbed;
 
 InternetSocket::InternetSocket()
     : _stack(0), _socket(0), _timeout(osWaitForever),
-      _readers(0), _writers(0), _pending(0),
+      _readers(0), _writers(0),
       _factory_allocated(false)
 {
+    core_util_atomic_flag_clear(&_pending);
     _socket_stats.stats_new_socket_entry(this);
 }
 
@@ -199,17 +200,16 @@ void InternetSocket::event()
 {
     _event_flag.set(READ_FLAG | WRITE_FLAG);
 
-    _pending += 1;
-    if (_callback && _pending == 1) {
+    if (_callback && !core_util_atomic_flag_test_and_set(&_pending)) {
         _callback();
     }
 }
 
 void InternetSocket::sigio(Callback<void()> callback)
 {
-    _lock.lock();
+    core_util_critical_section_enter();
     _callback = callback;
-    _lock.unlock();
+    core_util_critical_section_exit();
 }
 
 void InternetSocket::attach(Callback<void()> callback)
diff --git a/features/netsocket/InternetSocket.h b/features/netsocket/InternetSocket.h
@@ -25,6 +25,7 @@
 #include "rtos/Mutex.h"
 #include "rtos/EventFlags.h"
 #include "Callback.h"
+#include "mbed_critical.h"
 #include "mbed_toolchain.h"
 #include "SocketStats.h"
 
@@ -168,7 +169,7 @@ class InternetSocket : public Socket {
     SocketAddress _remote_peer;
     uint8_t _readers;
     uint8_t _writers;
-    volatile unsigned _pending;
+    core_util_atomic_flag _pending;
     bool _factory_allocated;
 
     // Event flags
diff --git a/features/netsocket/TCPServer.cpp b/features/netsocket/TCPServer.cpp
@@ -38,7 +38,7 @@ nsapi_error_t TCPServer::accept(TCPSocket *connection, SocketAddress *address)
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         void *socket;
         ret = _stack->socket_accept(_socket, &socket, address);
 
diff --git a/features/netsocket/TCPSocket.cpp b/features/netsocket/TCPSocket.cpp
@@ -62,7 +62,7 @@ nsapi_error_t TCPSocket::connect(const SocketAddress &address)
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         ret = _stack->socket_connect(_socket, address);
         if ((_timeout == 0) || !(ret == NSAPI_ERROR_IN_PROGRESS || ret == NSAPI_ERROR_ALREADY)) {
             _socket_stats.stats_update_socket_state(this, SOCK_CONNECTED);
@@ -143,7 +143,7 @@ nsapi_size_or_error_t TCPSocket::send(const void *data, nsapi_size_t size)
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         ret = _stack->socket_send(_socket, data_ptr + written, size - written);
         if (ret >= 0) {
             written += ret;
@@ -210,7 +210,7 @@ nsapi_size_or_error_t TCPSocket::recv(void *data, nsapi_size_t size)
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         ret = _stack->socket_recv(_socket, data, size);
         if ((_timeout == 0) || (ret != NSAPI_ERROR_WOULD_BLOCK)) {
             _socket_stats.stats_update_recv_bytes(this, ret);
@@ -281,7 +281,7 @@ TCPSocket *TCPSocket::accept(nsapi_error_t *error)
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         void *socket;
         SocketAddress address;
         ret = _stack->socket_accept(_socket, &socket, &address);
diff --git a/features/netsocket/UDPSocket.cpp b/features/netsocket/UDPSocket.cpp
@@ -70,7 +70,7 @@ nsapi_size_or_error_t UDPSocket::sendto(const SocketAddress &address, const void
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         nsapi_size_or_error_t sent = _stack->socket_sendto(_socket, address, data, size);
         if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != sent)) {
             _socket_stats.stats_update_sent_bytes(this, sent);
@@ -130,7 +130,7 @@ nsapi_size_or_error_t UDPSocket::recvfrom(SocketAddress *address, void *buffer,
             break;
         }
 
-        _pending = 0;
+        core_util_atomic_flag_clear(&_pending);
         nsapi_size_or_error_t recv = _stack->socket_recvfrom(_socket, address, buffer, size);
 
         // Filter incomming packets using connected peer address

Original file line number	Diff line number	Diff line change
`@@ -79,14 +79,6 @@ class stubInternetSocket : public InternetSocket {`
`79`	`79`	`{`
`80`	`80`	`_writers--;`
`81`	`81`	`}`
`82`		`- void add_pending(void)`
`83`		`- {`
`84`		`- _pending++;`
`85`		`- }`
`86`		`- void rem_pending(void)`
`87`		`- {`
`88`		`- _pending--;`
`89`		`- }`
`90`	`82`
`91`	`83`	`protected:`
`92`	`84`	`virtual nsapi_protocol_t get_proto()`
Original file line number	Diff line number	Diff line change
`@@ -21,9 +21,10 @@ using namespace mbed;`
`21`	`21`
`22`	`22`	`InternetSocket::InternetSocket()`
`23`	`23`	`: _stack(0), _socket(0), _timeout(osWaitForever),`
`24`		`- _readers(0), _writers(0), _pending(0),`
	`24`	`+ _readers(0), _writers(0),`
`25`	`25`	`_factory_allocated(false)`
`26`	`26`	`{`
	`27`	`+ core_util_atomic_flag_clear(&_pending);`
`27`	`28`	`_socket_stats.stats_new_socket_entry(this);`
`28`	`29`	`}`
`29`	`30`
`@@ -199,17 +200,16 @@ void InternetSocket::event()`
`199`	`200`	`{`
`200`	`201`	`_event_flag.set(READ_FLAG \| WRITE_FLAG);`
`201`	`202`
`202`		`- _pending += 1;`
`203`		`- if (_callback && _pending == 1) {`
	`203`	`+ if (_callback && !core_util_atomic_flag_test_and_set(&_pending)) {`
`204`	`204`	`_callback();`
`205`	`205`	`}`
`206`	`206`	`}`
`207`	`207`
`208`	`208`	`void InternetSocket::sigio(Callback<void()> callback)`
`209`	`209`	`{`
`210`		`- _lock.lock();`
	`210`	`+ core_util_critical_section_enter();`
`211`	`211`	`_callback = callback;`
`212`		`- _lock.unlock();`
	`212`	`+ core_util_critical_section_exit();`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`void InternetSocket::attach(Callback<void()> callback)`