Skip to content

Singleton support #2158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions hal/api/AnalogIn.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#if DEVICE_ANALOGIN

#include "analogin_api.h"
#include "SingletonPtr.h"

namespace mbed {

Expand Down Expand Up @@ -110,15 +111,15 @@ class AnalogIn {
protected:

virtual void lock() {
_mutex.lock();
_mutex->lock();
}

virtual void unlock() {
_mutex.unlock();
_mutex->unlock();
}

analogin_t _adc;
static PlatformMutex _mutex;
static SingletonPtr<PlatformMutex> _mutex;
};

} // namespace mbed
Expand Down
3 changes: 2 additions & 1 deletion hal/api/FileBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ typedef long off_t;
#endif

#include "platform.h"
#include "SingletonPtr.h"

namespace mbed {

Expand All @@ -65,7 +66,7 @@ class FileBase {
/* disallow copy constructor and assignment operators */
private:
static FileBase *_head;
static PlatformMutex _mutex;
static SingletonPtr<PlatformMutex> _mutex;

FileBase *_next;
const char * const _name;
Expand Down
3 changes: 2 additions & 1 deletion hal/api/I2C.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#if DEVICE_I2C

#include "i2c_api.h"
#include "SingletonPtr.h"

#if DEVICE_I2C_ASYNCH
#include "CThunk.h"
Expand Down Expand Up @@ -181,7 +182,7 @@ class I2C {
i2c_t _i2c;
static I2C *_owner;
int _hz;
static PlatformMutex _mutex;
static SingletonPtr<PlatformMutex> _mutex;
};

} // namespace mbed
Expand Down
103 changes: 103 additions & 0 deletions hal/api/SingletonPtr.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/* mbed Microcontroller Library
* Copyright (c) 2006-2013 ARM Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef SINGLETONPTR_H
#define SINGLETONPTR_H

#include <stdint.h>
#include <new>
#include "mbed_assert.h"
#ifdef MBED_CONF_RTOS_PRESENT
#include "cmsis_os.h"
#endif

#ifdef MBED_CONF_RTOS_PRESENT
extern osMutexId singleton_mutex_id;
#endif

/** Lock the singleton mutex
*
* This function is typically used to provide
* exclusive access when initializing a
* global object.
*/
inline static void singleton_lock(void)
{
#ifdef MBED_CONF_RTOS_PRESENT
osMutexWait(singleton_mutex_id, osWaitForever);
#endif
}

/** Unlock the singleton mutex
*
* This function is typically used to provide
* exclusive access when initializing a
* global object.
*/
inline static void singleton_unlock(void)
{
#ifdef MBED_CONF_RTOS_PRESENT
osMutexRelease (singleton_mutex_id);
#endif
}

/** Utility class for creating an using a singleton
*
* @Note Synchronization level: Thread safe
*
* @Note: This class must only be used in a static context -
* this class must never be allocated or created on the
* stack.
*
* @Note: This class is lazily initialized on first use.
* This class is a POD type so if it is not used it will
* be garbage collected.
*/
template <class T>
struct SingletonPtr {

/** Get a pointer to the underlying singleton
*
* @returns
* A pointer to the singleton
*/
T* get() {
if (NULL == _ptr) {
singleton_lock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do I understand this correctly, that any SinglePtr object would use one mutex singleton_mutex_id, means classes like SPI, AnalogIn would be "serialized"? If yes, what's the goal here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The singleton object is lazily created only when it is accessed. That way you can declare a class instance globally which gets garbage collected by the linker if it is unused.

The singleton_lock() is only for this initialization step, to prevent two threads from calling calling new on this object. Once the object is initialized the singleton mutex is no longer used.

_ptr = new (_data) T;
singleton_unlock();
}
// _ptr was not zero initialized or was
// corrupted if this assert is hit
MBED_ASSERT(_ptr == (T *)&_data);
return _ptr;
}

/** Get a pointer to the underlying singleton
*
* @returns
* A pointer to the singleton
*/
T* operator->() {
return get();
}

// This is zero initialized when in global scope
T *_ptr;
Copy link
Contributor

@bogdanm bogdanm Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't _ptr and _data private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be a plain old data (POD) type to get garbage collected by the linker properly. In C++ 03 POD type classes can't have any private members.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm, what @c1728p9 says is true until C++11 : (

// Force data to be 4 byte aligned
uint32_t _data[(sizeof(T) + sizeof(uint32_t) - 1) / sizeof(uint32_t)];
};

#endif
2 changes: 1 addition & 1 deletion hal/common/AnalogIn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

namespace mbed {

PlatformMutex AnalogIn::_mutex;
SingletonPtr<PlatformMutex> AnalogIn::_mutex;

};

Expand Down
22 changes: 11 additions & 11 deletions hal/common/FileBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,24 @@
namespace mbed {

FileBase *FileBase::_head = NULL;
PlatformMutex FileBase::_mutex;
SingletonPtr<PlatformMutex> FileBase::_mutex;

FileBase::FileBase(const char *name, PathType t) : _next(NULL),
_name(name),
_path_type(t) {
_mutex.lock();
_mutex->lock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These mutex lock()/unlock() sequences really need to be encapsulated in a RAII class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RAII would be good to have, as it would prevent the mutex from accidentally being left locked. It its not directly related to this PR though. Feel free to add a mutex RAII utility class in another PR.

if (name != NULL) {
// put this object at head of the list
_next = _head;
_head = this;
} else {
_next = NULL;
}
_mutex.unlock();
_mutex->unlock();
}

FileBase::~FileBase() {
_mutex.lock();
_mutex->lock();
if (_name != NULL) {
// remove this object from the list
if (_head == this) { // first in the list, so just drop me
Expand All @@ -48,38 +48,38 @@ FileBase::~FileBase() {
p->_next = _next;
}
}
_mutex.unlock();
_mutex->unlock();
}

FileBase *FileBase::lookup(const char *name, unsigned int len) {
_mutex.lock();
_mutex->lock();
FileBase *p = _head;
while (p != NULL) {
/* Check that p->_name matches name and is the correct length */
if (p->_name != NULL && std::strncmp(p->_name, name, len) == 0 && std::strlen(p->_name) == len) {
_mutex.unlock();
_mutex->unlock();
return p;
}
p = p->_next;
}
_mutex.unlock();
_mutex->unlock();
return NULL;
}

FileBase *FileBase::get(int n) {
_mutex.lock();
_mutex->lock();
FileBase *p = _head;
int m = 0;
while (p != NULL) {
if (m == n) {
_mutex.unlock();
_mutex->unlock();
return p;
}

m++;
p = p->_next;
}
_mutex.unlock();
_mutex->unlock();
return NULL;
}

Expand Down
6 changes: 3 additions & 3 deletions hal/common/I2C.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
namespace mbed {

I2C *I2C::_owner = NULL;
PlatformMutex I2C::_mutex;
SingletonPtr<PlatformMutex> I2C::_mutex;

I2C::I2C(PinName sda, PinName scl) :
#if DEVICE_I2C_ASYNCH
Expand Down Expand Up @@ -113,11 +113,11 @@ void I2C::stop(void) {
}

void I2C::lock() {
_mutex.lock();
_mutex->lock();
}

void I2C::unlock() {
_mutex.unlock();
_mutex->unlock();
}

#if DEVICE_I2C_ASYNCH
Expand Down
13 changes: 7 additions & 6 deletions hal/common/retarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "toolchain.h"
#include "semihost_api.h"
#include "mbed_interface.h"
#include "SingletonPtr.h"
#if DEVICE_STDIO_MESSAGES
#include <stdio.h>
#endif
Expand Down Expand Up @@ -72,17 +73,17 @@ extern const char __stderr_name[] = "/stderr";
* (or rather index+3, as filehandles 0-2 are stdin/out/err).
*/
static FileHandle *filehandles[OPEN_MAX];
static PlatformMutex filehandle_mutex;
static SingletonPtr<PlatformMutex> filehandle_mutex;

FileHandle::~FileHandle() {
filehandle_mutex.lock();
filehandle_mutex->lock();
/* Remove all open filehandles for this */
for (unsigned int fh_i = 0; fh_i < sizeof(filehandles)/sizeof(*filehandles); fh_i++) {
if (filehandles[fh_i] == this) {
filehandles[fh_i] = NULL;
}
}
filehandle_mutex.unlock();
filehandle_mutex->unlock();
}

#if DEVICE_SERIAL
Expand Down Expand Up @@ -162,17 +163,17 @@ extern "C" FILEHANDLE PREFIX(_open)(const char* name, int openmode) {
#endif

// find the first empty slot in filehandles
filehandle_mutex.lock();
filehandle_mutex->lock();
unsigned int fh_i;
for (fh_i = 0; fh_i < sizeof(filehandles)/sizeof(*filehandles); fh_i++) {
if (filehandles[fh_i] == NULL) break;
}
if (fh_i >= sizeof(filehandles)/sizeof(*filehandles)) {
filehandle_mutex.unlock();
filehandle_mutex->unlock();
return -1;
}
filehandles[fh_i] = (FileHandle*)FILE_HANDLE_RESERVED;
filehandle_mutex.unlock();
filehandle_mutex->unlock();

FileHandle *res;

Expand Down
32 changes: 14 additions & 18 deletions rtos/rtx/TARGET_CORTEX_A/RTX_CM_lib.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
*---------------------------------------------------------------------------*/

#if defined (__CC_ARM)
#include <rt_misc.h>
#pragma O3
#define __USED __attribute__((used))
#elif defined (__GNUC__)
Expand Down Expand Up @@ -224,6 +225,10 @@ uint32_t os_tmr = 0;
uint32_t const *m_tmr = NULL;
uint16_t const mp_tmr_size = 0;

/* singleton mutex */
osMutexId singleton_mutex_id;
osMutexDef(singleton_mutex);

#if defined (__CC_ARM) && !defined (__MICROLIB)
/* A memory space for arm standard library. */
static uint32_t std_libspace[OS_TASK_CNT][96/4];
Expand Down Expand Up @@ -433,6 +438,7 @@ void $Sub$$__cpp_initialize__aeabi_(void)

void pre_main()
{
singleton_mutex_id = osMutexCreate(osMutex(singleton_mutex));
$Super$$__cpp_initialize__aeabi_();
main();
}
Expand All @@ -442,25 +448,13 @@ void pre_main()
void * armcc_heap_base;
void * armcc_heap_top;

__asm void pre_main (void)
{
IMPORT __rt_lib_init
IMPORT main
IMPORT armcc_heap_base
IMPORT armcc_heap_top
int main(void);

LDR R0,=armcc_heap_base
LDR R1,=armcc_heap_top
LDR R0,[R0]
LDR R1,[R1]
/* Save link register (keep 8 byte alignment with dummy R4) */
PUSH {R4, LR}
BL __rt_lib_init
BL main
/* Return to the thread destroy function.
*/
POP {R4, PC}
ALIGN
void pre_main (void)
{
singleton_mutex_id = osMutexCreate(osMutex(singleton_mutex));
__rt_lib_init((unsigned)armcc_heap_base, (unsigned)armcc_heap_top);
Copy link
Contributor

@bogdanm bogdanm Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it's OK to call osMutexCreate before __rt_lib_init? How was this tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep this is safe to call here. The RTOS has been initialized by the time pre_main is called. The function pre_main its actually running on an RTOS created thread. The function __rt_lib_init is responsible for finishing up the standard library initialization by calling constructors.

main();
}

__asm void __rt_entry (void) {
Expand Down Expand Up @@ -496,6 +490,7 @@ extern void __libc_init_array (void);
extern int main(int argc, char **argv);

void pre_main(void) {
singleton_mutex_id = osMutexCreate(osMutex(singleton_mutex));
atexit(__libc_fini_array);
__libc_init_array();
main(0, NULL);
Expand Down Expand Up @@ -528,6 +523,7 @@ extern int main(void);
static uint8_t low_level_init_needed;

void pre_main(void) {
singleton_mutex_id = osMutexCreate(osMutex(singleton_mutex));
if (low_level_init_needed) {
__iar_dynamic_initialization();
}
Expand Down
Loading