enforce the thread safety of the tbm_surface 17/89517/1
authorChangyeon Lee <cyeon.lee@samsung.com>
Sun, 25 Sep 2016 07:04:26 +0000 (16:04 +0900)
committerChangyeon Lee <cyeon.lee@samsung.com>
Sun, 25 Sep 2016 10:17:56 +0000 (19:17 +0900)
Change-Id: I08b1a98ef2e49382d709632a6a9beb575fc21fa4

src/tbm_surface.c
src/tbm_surface_internal.c

index 4a36500..1277c80 100644 (file)
@@ -78,8 +78,7 @@ tbm_surface_destroy(tbm_surface_h surface)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       if (!tbm_surface_internal_is_valid(surface))
-               return TBM_SURFACE_ERROR_INVALID_PARAMETER;
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        tbm_surface_internal_destroy(surface);
 
@@ -91,7 +90,7 @@ tbm_surface_map(tbm_surface_h surface, int opt, tbm_surface_info_s *info)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), TBM_SURFACE_ERROR_INVALID_PARAMETER);
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
        TBM_RETURN_VAL_IF_FAIL(info != NULL, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        int ret = 0;
@@ -108,7 +107,7 @@ tbm_surface_unmap(tbm_surface_h surface)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), TBM_SURFACE_ERROR_INVALID_PARAMETER);
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        tbm_surface_internal_unmap(surface);
 
@@ -120,7 +119,7 @@ tbm_surface_get_info(tbm_surface_h surface, tbm_surface_info_s *info)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), TBM_SURFACE_ERROR_INVALID_PARAMETER);
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
        TBM_RETURN_VAL_IF_FAIL(info != NULL, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        int ret = 0;
@@ -137,7 +136,7 @@ tbm_surface_get_width(tbm_surface_h surface)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), TBM_SURFACE_ERROR_INVALID_PARAMETER);
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        return tbm_surface_internal_get_width(surface);
 }
@@ -147,7 +146,7 @@ tbm_surface_get_height(tbm_surface_h surface)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), TBM_SURFACE_ERROR_INVALID_PARAMETER);
+       TBM_RETURN_VAL_IF_FAIL(surface, TBM_SURFACE_ERROR_INVALID_PARAMETER);
 
        return tbm_surface_internal_get_height(surface);
 }
@@ -157,7 +156,7 @@ tbm_surface_get_format(tbm_surface_h surface)
 {
        TBM_TRACE("tbm_surface(%p)\n", surface);
 
-       if (!tbm_surface_internal_is_valid(surface)) {
+       if (!surface) {
 #ifdef HAVE_CAPI_0_1_1
                set_last_result(TBM_SURFACE_ERROR_INVALID_PARAMETER);
 #endif
index 57c491f..39ebb28 100644 (file)
@@ -39,14 +39,32 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "list.h"
 #include <png.h>
 
+static tbm_bufmgr g_surface_bufmgr;
+static pthread_mutex_t tbm_surface_lock;
+void _tbm_surface_mutex_unlock(void);
+
 #define C(b, m)              (((b) >> (m)) & 0xFF)
 #define B(c, s)              ((((unsigned int)(c)) & 0xff) << (s))
 #define FOURCC(a, b, c, d)     (B(d, 24) | B(c, 16) | B(b, 8) | B(a, 0))
 #define FOURCC_STR(id)      C(id, 0), C(id, 8), C(id, 16), C(id, 24)
 #define FOURCC_ID(str)      FOURCC(((char*)str)[0], ((char*)str)[1], ((char*)str)[2], ((char*)str)[3])
 
-static tbm_bufmgr g_surface_bufmgr;
-static pthread_mutex_t tbm_surface_lock;
+/* check condition */
+#define TBM_SURFACE_RETURN_IF_FAIL(cond) {\
+       if (!(cond)) {\
+               TBM_LOG_E("'%s' failed.\n", #cond);\
+               _tbm_surface_mutex_unlock();\
+               return;\
+       } \
+}
+
+#define TBM_SURFACE_RETURN_VAL_IF_FAIL(cond, val) {\
+       if (!(cond)) {\
+               TBM_LOG_E("'%s' failed.\n", #cond);\
+               _tbm_surface_mutex_unlock();\
+               return val;\
+       } \
+}
 
 /* LCOV_EXCL_START */
 #define USE_REALTIME 1
@@ -828,11 +846,10 @@ bail1:
 void
 tbm_surface_internal_destroy(tbm_surface_h surface)
 {
-       if (!tbm_surface_internal_is_valid(surface))
-               return;
-
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
+
        surface->refcnt--;
 
        if (surface->refcnt > 0) {
@@ -852,10 +869,10 @@ tbm_surface_internal_destroy(tbm_surface_h surface)
 void
 tbm_surface_internal_ref(tbm_surface_h surface)
 {
-       TBM_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
-
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
+
        surface->refcnt++;
 
        TBM_TRACE("tbm_surface(%p) refcnt(%d)\n", surface, surface->refcnt);
@@ -866,10 +883,10 @@ tbm_surface_internal_ref(tbm_surface_h surface)
 void
 tbm_surface_internal_unref(tbm_surface_h surface)
 {
-       TBM_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
-
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
+
        surface->refcnt--;
 
        if (surface->refcnt > 0) {
@@ -889,13 +906,13 @@ tbm_surface_internal_unref(tbm_surface_h surface)
 int
 tbm_surface_internal_get_num_bos(tbm_surface_h surface)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        int num;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        surf = (struct _tbm_surface *)surface;
        num = surf->num_bos;
 
@@ -909,14 +926,14 @@ tbm_surface_internal_get_num_bos(tbm_surface_h surface)
 tbm_bo
 tbm_surface_internal_get_bo(tbm_surface_h surface, int bo_idx)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), NULL);
-       TBM_RETURN_VAL_IF_FAIL(bo_idx > -1, NULL);
-
        struct _tbm_surface *surf;
        tbm_bo bo;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), NULL);
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(bo_idx > -1, NULL);
+
        surf = (struct _tbm_surface *)surface;
        bo = surf->bos[bo_idx];
 
@@ -930,13 +947,13 @@ tbm_surface_internal_get_bo(tbm_surface_h surface, int bo_idx)
 int
 tbm_surface_internal_get_size(tbm_surface_h surface)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        unsigned int size;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        surf = (struct _tbm_surface *)surface;
        size = surf->info.size;
 
@@ -951,13 +968,13 @@ int
 tbm_surface_internal_get_plane_data(tbm_surface_h surface, int plane_idx,
                                    uint32_t *size, uint32_t *offset, uint32_t *pitch)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-       TBM_RETURN_VAL_IF_FAIL(plane_idx > -1, 0);
-
        struct _tbm_surface *surf;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(plane_idx > -1, 0);
+
        surf = (struct _tbm_surface *)surface;
 
        if (plane_idx >= surf->info.num_planes) {
@@ -988,14 +1005,14 @@ int
 tbm_surface_internal_get_info(tbm_surface_h surface, int opt,
                              tbm_surface_info_s *info, int map)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        tbm_bo_handle bo_handles[4];
        int i, j;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        memset(bo_handles, 0, sizeof(tbm_bo_handle) * 4);
 
        surf = (struct _tbm_surface *)surface;
@@ -1045,13 +1062,13 @@ tbm_surface_internal_get_info(tbm_surface_h surface, int opt,
 void
 tbm_surface_internal_unmap(tbm_surface_h surface)
 {
-       TBM_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
-
        struct _tbm_surface *surf;
        int i;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
+
        surf = (struct _tbm_surface *)surface;
 
        for (i = 0; i < surf->num_bos; i++)
@@ -1065,13 +1082,13 @@ tbm_surface_internal_unmap(tbm_surface_h surface)
 unsigned int
 tbm_surface_internal_get_width(tbm_surface_h surface)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        unsigned int width;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        surf = (struct _tbm_surface *)surface;
        width = surf->info.width;
 
@@ -1085,13 +1102,13 @@ tbm_surface_internal_get_width(tbm_surface_h surface)
 unsigned int
 tbm_surface_internal_get_height(tbm_surface_h surface)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        unsigned int height;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        surf = (struct _tbm_surface *)surface;
        height = surf->info.height;
 
@@ -1106,13 +1123,13 @@ tbm_surface_internal_get_height(tbm_surface_h surface)
 tbm_format
 tbm_surface_internal_get_format(tbm_surface_h surface)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        struct _tbm_surface *surf;
        tbm_format format;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        surf = (struct _tbm_surface *)surface;
        format = surf->info.format;
 
@@ -1126,13 +1143,14 @@ tbm_surface_internal_get_format(tbm_surface_h surface)
 int
 tbm_surface_internal_get_plane_bo_idx(tbm_surface_h surface, int plane_idx)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-       TBM_RETURN_VAL_IF_FAIL(plane_idx > -1, 0);
        struct _tbm_surface *surf;
        int bo_idx;
 
        _tbm_surface_mutex_lock();
 
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(plane_idx > -1, 0);
+
        surf = (struct _tbm_surface *)surface;
        bo_idx = surf->planes_bo_idx[plane_idx];
 
@@ -1147,20 +1165,24 @@ int
 tbm_surface_internal_add_user_data(tbm_surface_h surface, unsigned long key,
                                   tbm_data_free data_free_func)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        tbm_user_data *data;
 
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        /* check if the data according to the key exist if so, return false. */
        data = user_data_lookup(&surface->user_data_list, key);
        if (data) {
                TBM_TRACE("warning: user data already exist tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
        data = user_data_create(key, data_free_func);
        if (!data) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1168,6 +1190,8 @@ tbm_surface_internal_add_user_data(tbm_surface_h surface, unsigned long key,
 
        LIST_ADD(&data->item_link, &surface->user_data_list);
 
+       _tbm_surface_mutex_unlock();
+
        return 1;
 }
 
@@ -1175,18 +1199,22 @@ int
 tbm_surface_internal_set_user_data(tbm_surface_h surface, unsigned long key,
                                   void *data)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        tbm_user_data *old_data;
 
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        if (LIST_IS_EMPTY(&surface->user_data_list)) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
        old_data = user_data_lookup(&surface->user_data_list, key);
        if (!old_data) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1197,6 +1225,8 @@ tbm_surface_internal_set_user_data(tbm_surface_h surface, unsigned long key,
 
        TBM_TRACE("tbm_surface(%p) key(%lu) data(%p)\n", surface, key, old_data->data);
 
+       _tbm_surface_mutex_unlock();
+
        return 1;
 }
 
@@ -1204,12 +1234,15 @@ int
 tbm_surface_internal_get_user_data(tbm_surface_h surface, unsigned long key,
                                   void **data)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        tbm_user_data *old_data;
 
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        if (!data || LIST_IS_EMPTY(&surface->user_data_list)) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1217,6 +1250,7 @@ tbm_surface_internal_get_user_data(tbm_surface_h surface, unsigned long key,
        if (!old_data) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
                *data = NULL;
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1224,6 +1258,8 @@ tbm_surface_internal_get_user_data(tbm_surface_h surface, unsigned long key,
 
        TBM_TRACE("tbm_surface(%p) key(%lu) data(%p)\n", surface, key, old_data->data);
 
+       _tbm_surface_mutex_unlock();
+
        return 1;
 }
 
@@ -1231,18 +1267,22 @@ int
 tbm_surface_internal_delete_user_data(tbm_surface_h surface,
                                      unsigned long key)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-
        tbm_user_data *old_data = (void *)0;
 
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+
        if (LIST_IS_EMPTY(&surface->user_data_list)) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
        old_data = user_data_lookup(&surface->user_data_list, key);
        if (!old_data) {
                TBM_TRACE("error: tbm_surface(%p) key(%lu)\n", surface, key);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1250,6 +1290,8 @@ tbm_surface_internal_delete_user_data(tbm_surface_h surface,
 
        user_data_delete(old_data);
 
+       _tbm_surface_mutex_unlock();
+
        return 1;
 }
 
@@ -1265,9 +1307,13 @@ _tbm_surface_internal_get_debug_pid(tbm_surface_h surface)
 void
 tbm_surface_internal_set_debug_pid(tbm_surface_h surface, unsigned int pid)
 {
-       TBM_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_IF_FAIL(tbm_surface_internal_is_valid(surface));
 
        surface->debug_pid = pid;
+
+       _tbm_surface_mutex_unlock();
 }
 
 static tbm_surface_debug_data *
@@ -1288,14 +1334,15 @@ _tbm_surface_internal_debug_data_create(char *key, char *value)
 int
 tbm_surface_internal_set_debug_data(tbm_surface_h surface, char *key, char *value)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
-       TBM_RETURN_VAL_IF_FAIL(key, 0);
-
        tbm_surface_debug_data *debug_data = NULL;
        tbm_surface_debug_data *old_data = NULL, *tmp = NULL;
        tbm_bufmgr bufmgr = surface->bufmgr;
 
-       TBM_RETURN_VAL_IF_FAIL(bufmgr, 0);
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), 0);
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(key, 0);
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(bufmgr, 0);
 
        if (!LIST_IS_EMPTY(&surface->debug_data_list)) {
                LIST_FOR_EACH_ENTRY_SAFE(old_data, tmp, &surface->debug_data_list, item_link) {
@@ -1311,6 +1358,7 @@ tbm_surface_internal_set_debug_data(tbm_surface_h surface, char *key, char *valu
        debug_data = _tbm_surface_internal_debug_data_create(key, value);
        if (!debug_data) {
                TBM_TRACE("error: tbm_surface(%p) key(%s) value(%s)\n", surface, key, value);
+               _tbm_surface_mutex_unlock();
                return 0;
        }
 
@@ -1320,31 +1368,40 @@ tbm_surface_internal_set_debug_data(tbm_surface_h surface, char *key, char *valu
 
        if (!LIST_IS_EMPTY(&bufmgr->debug_key_list)) {
                LIST_FOR_EACH_ENTRY_SAFE(old_data, tmp, &bufmgr->debug_key_list, item_link) {
-                       if (!strcmp(old_data->key, key))
+                       if (!strcmp(old_data->key, key)) {
+                               _tbm_surface_mutex_unlock();
                                return 1;
+                       }
                }
        }
 
        debug_data = _tbm_surface_internal_debug_data_create(key, NULL);
        LIST_ADD(&debug_data->item_link, &bufmgr->debug_key_list);
 
+       _tbm_surface_mutex_unlock();
+
        return 1;
 }
 
 char *
 _tbm_surface_internal_get_debug_data(tbm_surface_h surface, char *key)
 {
-       TBM_RETURN_VAL_IF_FAIL(tbm_surface_internal_is_valid(surface), NULL);
-
        tbm_surface_debug_data *old_data = NULL, *tmp = NULL;
 
+       _tbm_surface_mutex_lock();
+
+       TBM_SURFACE_RETURN_VAL_IF_FAIL(surface, NULL);
+
        if (!LIST_IS_EMPTY(&surface->debug_data_list)) {
                LIST_FOR_EACH_ENTRY_SAFE(old_data, tmp, &surface->debug_data_list, item_link) {
-                       if (!strcmp(old_data->key, key))
+                       if (!strcmp(old_data->key, key)) {
                                return old_data->value;
+                       }
                }
        }
 
+       _tbm_surface_mutex_unlock();
+
        return NULL;
 }