diff --git a/src/drivers/device/device_nuttx.h b/src/drivers/device/device_nuttx.h index c8f732fab1..b36b58b4ad 100644 --- a/src/drivers/device/device_nuttx.h +++ b/src/drivers/device/device_nuttx.h @@ -188,6 +188,8 @@ protected: * Each driver instance has its own lock/semaphore. * * Note that we must loop as the wait may be interrupted by a signal. + * + * Careful: lock() calls cannot be nested! */ void lock() { @@ -202,10 +204,11 @@ protected: sem_post(&_lock); } + sem_t _lock; /**< lock to protect access to all class members (also for derived classes) */ + private: int _irq; bool _irq_attached; - sem_t _lock; /** disable copy construction for this and all subclasses */ Device(const Device &); diff --git a/src/drivers/device/vdev.h b/src/drivers/device/vdev.h index 3875ea7706..a5d76ce61a 100644 --- a/src/drivers/device/vdev.h +++ b/src/drivers/device/vdev.h @@ -196,29 +196,9 @@ protected: px4_sem_post(&_lock); } - /** - * @class Smart locking object that uses the same lock as lock(), but automatically - * takes the lock when created and releases the lock when the object goes out of - * scope. Use like this: - * { - * SmartLock smart_lock(*this); - * //critical section start - * ... - * //critical section end - * } - */ - class SmartLock - { - public: - SmartLock(Device &device) : _device(device) { _device.lock(); } - ~SmartLock() { _device.unlock(); } - private: - Device &_device; - }; - -private: px4_sem_t _lock; /**< lock to protect access to all class members (also for derived classes) */ +private: /** disable copy construction for this and all subclasses */ Device(const Device &); diff --git a/src/modules/uORB/uORBDevices_nuttx.cpp b/src/modules/uORB/uORBDevices_nuttx.cpp index 3e254da8c1..d9271d085b 100644 --- a/src/modules/uORB/uORBDevices_nuttx.cpp +++ b/src/modules/uORB/uORBDevices_nuttx.cpp @@ -40,6 +40,7 @@ #include "uORBUtils.hpp" #include "uORBManager.hpp" #include "uORBCommunicator.hpp" +#include #include uORB::ORBMap uORB::DeviceMaster::_node_map; @@ -603,9 +604,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) return ret; } - /* ensure that only one advertiser runs through this critical section */ - lock(); - ret = ERROR; /* try for topic groups */ @@ -619,11 +617,12 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) group_tries = *adv->instance; if (group_tries >= max_group_tries) { - unlock(); return -ENOMEM; } } + SmartLock smart_lock(_lock); + do { /* if path is modifyable change try index */ if (adv->instance != nullptr) { @@ -636,7 +635,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) objname = strdup(meta->o_name); if (objname == nullptr) { - unlock(); return -ENOMEM; } @@ -644,7 +642,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) devpath = strdup(nodepath); if (devpath == nullptr) { - unlock(); free((void *)objname); return -ENOMEM; } @@ -654,7 +651,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) /* if we didn't get a device, that's bad */ if (node == nullptr) { - unlock(); free((void *)objname); free((void *)devpath); return -ENOMEM; @@ -698,9 +694,6 @@ uORB::DeviceMaster::ioctl(struct file *filp, int cmd, unsigned long arg) ret = -ENOMEM; } - /* the file handle for the driver has been created, unlock */ - unlock(); - return ret; } diff --git a/src/modules/uORB/uORBDevices_posix.cpp b/src/modules/uORB/uORBDevices_posix.cpp index 854a68e402..d641859875 100644 --- a/src/modules/uORB/uORBDevices_posix.cpp +++ b/src/modules/uORB/uORBDevices_posix.cpp @@ -41,6 +41,7 @@ #include "uORBUtils.hpp" #include "uORBManager.hpp" #include "uORBCommunicator.hpp" +#include #include std::map uORB::DeviceMaster::_node_map; @@ -629,7 +630,7 @@ uORB::DeviceMaster::ioctl(device::file_t *filp, int cmd, unsigned long arg) } } - SmartLock smart_lock(*this); + SmartLock smart_lock(_lock); do { /* if path is modifyable change try index */ diff --git a/src/platforms/px4_sem.hpp b/src/platforms/px4_sem.hpp new file mode 100644 index 0000000000..17aca78af5 --- /dev/null +++ b/src/platforms/px4_sem.hpp @@ -0,0 +1,68 @@ +/**************************************************************************** + * + * Copyright (c) 2016 PX4 Development Team. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * 3. Neither the name PX4 nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN + * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + ****************************************************************************/ + +/** + * @file px4_sem.hpp + * + * C++ synchronization helpers + */ + +#pragma once + +#include "px4_sem.h" + + +/** + * @class Smart locking object that uses a semaphore. It automatically + * takes the lock when created and releases the lock when the object goes out of + * scope. Use like this: + * + * px4_sem_t my_lock; + * int ret = px4_sem_init(&my_lock, 0, 1); + * ... + * + * { + * SmartLock smart_lock(my_lock); + * //critical section start + * ... + * //critical section end + * } + */ +class SmartLock +{ +public: + SmartLock(px4_sem_t &sem) : _sem(sem) { do {} while (px4_sem_wait(&_sem) != 0); } + ~SmartLock() { px4_sem_post(&_sem); } +private: + px4_sem_t &_sem; +};