A new version of the patch wit eina_threads_init() and
eina_threads_shutdown(). maybe we can add a call to these functions in
ecore_thread_run () ?
2009/10/28 Atton Jonathan <jonathan.at...@gmail.com>
>
> I have attached the patch.
>
> With the help of Cedric I have made this patch.
>
> 2 mutexs are used, one for the strings with a size < 4 and one for the
> others.
> 2 macros has been updated to add the possibility to unlock a mutex if
> necessary.
> In this version the mutexs are activated if eina is installed with the
> pthread support ( I use the same code than eina_log ). If you wish I can add
> a function to manually active the mutexs.
>
> The patch is simple, this was not a big job to do. Helgrind reports no
> errors with my application and I use a lot of eina stringshare + threads
> (without the patch my application segv after eina reports some invalids use
> of eina stringshare). Of course I do not use all the combinations of calls.
> E17 works too.
>
>
> 2009/10/26 Cedric BAIL <cedric.b...@free.fr>
>
> On Mon, Oct 26, 2009 at 1:07 PM, Atton Jonathan
>> <jonathan.at...@gmail.com> wrote:
>> > actually all eina should be thread safe. Maybe eet and ecore too. If you
>> are
>> > not against this I will start to work on eina.
>>
>> Actually, I think we should do this really carefully. I think we
>> should make stringshare thread safe, because their is no way from an
>> user API point of view to use them without messing everything from
>> another thread.
>>
>> So i agree we will need something like 'int
>> eina_thread_safe_init(void)' and 'int
>> eina_thread_safe_shutdown(void)', that track the number of time an
>> user need to be safe and set one global flag that activate some mutex
>> for stringshare. But I don't want to see eina_list or eina_hash, add
>> some pthread lock just because the user can't do it by itself. And in
>> most case, he will be able to do it much more efficiently than we
>> could in eina.
>>
>> > 2009/10/25 Gustavo Sverzut Barbieri <barbi...@profusion.mobi>
>> >> On Sun, Oct 25, 2009 at 5:20 PM, Atton Jonathan
>> >> <jonathan.at...@gmail.com> wrote:
>> >> > hey,
>> >> >
>> >> > I talked a week ago with cedric about making eina stringshare thread
>> >> safe.
>> >> >
>> >> > The mutex could be activated/deactivated with a classic function and
>> then
>> >> 2
>> >> > macros will be use:
>> >> >
>> >> > LOCK : if(mutex_activated) pthread_mutex_lock()
>> >> > UNLOCK : if(mutex_activated) pthread_mutex_unlock()
>> >> >
>> >> > By default the mutex can be deactivated and the developer activate it
>> if
>> >> > necessary.
>> >> >
>> >> > What do you think about this idea ?
>> >>
>> >> The only problem with this is inconsistency. Okay, we have stringshare
>> >> as this, but still default mempool is not so you cannot allocate list
>> >> nodes... then you cannot add ecore idlers/timers/pollers/... from
>> >> threads, and so on. Inconsistency is bad :-(
>>
>> The default mempool is thread safe. Definitivly, it should work safely
>> if you build eina with pthread support. If not, it's a bug that should
>> be fixed. For ecore, this is another matter, right now, by design we
>> don't want to
>>
>> >> I really don't see any clear solution to this problem. Maybe we could
>> >> have something like glib's "threads init". I have this for logging,
>> >> but i was really reluctant to add it. But if so, then we must make
>> >> sure all types would be thread safe.
>>
>> So maybe when thread safe is required do the same trick for eina_log to.
>> --
>> Cedric BAIL
>>
>
>
>
> --
> Regards.
>
--
Regards.
Index: src/include/eina_private.h
===================================================================
--- src/include/eina_private.h (revision 43297)
+++ src/include/eina_private.h (working copy)
@@ -116,5 +116,10 @@
} \
} while(0);
+#ifdef EFL_HAVE_PTHREAD
+void eina_stringshare_threads_init(void);
+void eina_stringshare_threads_shutdown(void);
+#endif
+
#endif /* EINA_PRIVATE_H_ */
Index: src/include/eina_main.h
===================================================================
--- src/include/eina_main.h (revision 43297)
+++ src/include/eina_main.h (working copy)
@@ -35,6 +35,10 @@
EAPI int eina_shutdown(void);
+EAPI int eina_threads_init(void);
+
+EAPI int eina_threads_shutdown(void);
+
/**
* @}
*/
Index: src/lib/eina_log.c
===================================================================
--- src/lib/eina_log.c (revision 43297)
+++ src/lib/eina_log.c (working copy)
@@ -1391,3 +1391,4 @@
eina_log_print_unlocked(domain, level, file, fnc, line, fmt, args);
UNLOCK();
}
+
Index: src/lib/eina_main.c
===================================================================
--- src/lib/eina_main.c (revision 43297)
+++ src/lib/eina_main.c (working copy)
@@ -48,6 +48,7 @@
*/
static int _eina_main_count = 0;
+static int _eina_main_thread_count = 0;
static int _eina_log_dom = -1;
#ifdef ERR
@@ -60,6 +61,16 @@
#endif
#define DBG(...) EINA_LOG_DOM_DBG(_eina_log_dom, __VA_ARGS__)
+#ifdef EFL_HAVE_PTHREAD
+#include <pthread.h>
+static Eina_Bool _threads_activated = EINA_FALSE;
+static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER;
+#define LOCK() if(_threads_activated) pthread_mutex_lock(&_mutex);
+#define UNLOCK() if(_threads_activated) pthread_mutex_unlock(&_mutex);
+#else
+#define LOCK() do {} while (0)
+#define UNLOCK() do {} while (0)
+#endif
/* place module init/shutdown functions here to avoid other modules
* calling them by mistake.
@@ -220,6 +231,84 @@
return _eina_main_count;
}
+
/**
+ * @brief Initialize the mutexs of the Eina library.
+ *
+ * @return 1 or greater on success, 0 on error.
+ *
+ * This function sets up all the mutexs in all eina modules. It returns 0 on
+ * failure (that is, when one of the module fails to initialize),
+ * otherwise it returns the number of times it has already been
+ * called.
+ *
+ * When the mutexs are not used anymore, call eina_thread_shutdown() to shut
down
+ * the mutexs.
+ */
+EAPI int
+eina_threads_init(void)
+{
+#ifdef EFL_HAVE_PTHREAD
+ int ret;
+
+ LOCK();
+ ++_eina_main_thread_count;
+ ret = _eina_main_thread_count;
+
+ if(_eina_main_thread_count > 1)
+ {
+ UNLOCK();
+ return ret;
+ }
+
+ eina_stringshare_threads_init();
+ _threads_activated = EINA_TRUE;
+
+ return ret;
+#else
+ return 0;
+#endif
+}
+
+/**
+ * @brief Shut down mutexs in the Eina library.
+ *
+ * @return 0 when all mutexs are completely shut down, 1 or
+ * greater otherwise.
+ *
+ * This function shuts down the mutexs in the Eina library. It returns 0 when
it has
+ * been called the same number of times than eina_thread_init(). In that case
+ * it shut down all the mutexs.
+ *
+ * Once this function succeeds (that is, @c 0 is returned), you must
+ * not call any of the Eina function in a thread anymore. You must call
+ * eina_thread_init() again to use the Eina functions in a thread again.
+ */
+EAPI int
+eina_threads_shutdown(void)
+{
+#ifdef EFL_HAVE_PTHREAD
+ int ret;
+
+ LOCK();
+ ret = --_eina_main_thread_count;
+ if(_eina_main_thread_count > 0)
+ {
+ UNLOCK();
+ return ret;
+ }
+
+ eina_stringshare_threads_shutdown();
+ _threads_activated = EINA_FALSE;
+ UNLOCK();
+
+ return ret;
+#else
+ return 0;
+#endif
+}
+
+
+/**
* @}
*/
Index: src/lib/eina_stringshare.c
===================================================================
--- src/lib/eina_stringshare.c (revision 43297)
+++ src/lib/eina_stringshare.c (working copy)
@@ -100,19 +100,23 @@
static const char EINA_MAGIC_STRINGSHARE_NODE_STR[] = "Eina Stringshare Node";
-#define EINA_MAGIC_CHECK_STRINGSHARE_HEAD(d, ...) \
+#define EINA_MAGIC_CHECK_STRINGSHARE_HEAD(d, unlock, ...) \
do { \
if (!EINA_MAGIC_CHECK((d), EINA_MAGIC_STRINGSHARE_HEAD)) \
{ \
EINA_MAGIC_FAIL((d), EINA_MAGIC_STRINGSHARE_HEAD); \
+ unlock; \
return __VA_ARGS__; \
} \
} while (0);
-#define EINA_MAGIC_CHECK_STRINGSHARE_NODE(d) \
+#define EINA_MAGIC_CHECK_STRINGSHARE_NODE(d, unlock) \
do { \
if (!EINA_MAGIC_CHECK((d), EINA_MAGIC_STRINGSHARE_NODE)) \
+ { \
+ unlock; \
EINA_MAGIC_FAIL((d), EINA_MAGIC_STRINGSHARE_NODE); \
+ } \
} while (0);
typedef struct _Eina_Stringshare Eina_Stringshare;
@@ -170,6 +174,27 @@
#endif
#define DBG(...) EINA_LOG_DOM_DBG(_eina_stringshare_log_dom, __VA_ARGS__)
+
+
+#ifdef EFL_HAVE_PTHREAD
+#include <pthread.h>
+static Eina_Bool _threads_activated = EINA_FALSE;
+//string < 4
+static pthread_mutex_t _mutex_small = PTHREAD_MUTEX_INITIALIZER;
+//string >= 4
+static pthread_mutex_t _mutex_big = PTHREAD_MUTEX_INITIALIZER;
+#define LOCK_SMALL() if(_threads_activated) pthread_mutex_lock(&_mutex_small);
+#define UNLOCK_SMALL() if(_threads_activated)
pthread_mutex_unlock(&_mutex_small);
+#define LOCK_BIG() if(_threads_activated) pthread_mutex_lock(&_mutex_big);
+#define UNLOCK_BIG() if(_threads_activated) pthread_mutex_unlock(&_mutex_big);
+#else
+#define LOCK_SMALL() do {} while (0)
+#define UNLOCK_SMALL() do {} while (0)
+#define LOCK_BIG() do {} while (0)
+#define UNLOCK_BIG() do {} while (0)
+#endif
+
+
static const unsigned char _eina_stringshare_single[512] = {
0,0,1,0,2,0,3,0,4,0,5,0,6,0,7,0,8,0,9,0,10,0,11,0,12,0,13,0,14,0,15,0,
16,0,17,0,18,0,19,0,20,0,21,0,22,0,23,0,24,0,25,0,26,0,27,0,28,0,29,0,30,0,
@@ -276,6 +301,9 @@
static void
_eina_stringshare_population_add(int slen)
{
+ LOCK_SMALL();
+ LOCK_BIG();
+
population.count++;
if (population.count > population.max)
population.max = population.count;
@@ -286,14 +314,23 @@
if (population_group[slen].count > population_group[slen].max)
population_group[slen].max = population_group[slen].count;
}
+
+ UNLOCK_BIG();
+ UNLOCK_SMALL();
}
static void
_eina_stringshare_population_del(int slen)
{
+ LOCK_SMALL();
+ LOCK_BIG();
+
population.count--;
if (slen < 4)
population_group[slen].count--;
+
+ UNLOCK_BIG();
+ UNLOCK_SMALL();
}
static void
@@ -331,7 +368,7 @@
static int
_eina_stringshare_cmp(const Eina_Stringshare_Head *ed, const int *hash,
__UNUSED__ int length, __UNUSED__ void *data)
{
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, 0);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, , 0);
return ed->hash - *hash;
}
@@ -339,8 +376,8 @@
static Eina_Rbtree_Direction
_eina_stringshare_node(const Eina_Stringshare_Head *left, const
Eina_Stringshare_Head *right, __UNUSED__ void *data)
{
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(left, 0);
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(right, 0);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(left, , 0);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(right, , 0);
if (left->hash - right->hash < 0)
return EINA_RBTREE_LEFT;
@@ -350,7 +387,7 @@
static void
_eina_stringshare_head_free(Eina_Stringshare_Head *ed, __UNUSED__ void *data)
{
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, );
while (ed->head)
{
@@ -879,6 +916,9 @@
{
unsigned int i;
+ LOCK_SMALL();
+ LOCK_BIG();
+
_eina_stringshare_population_stats();
/* remove any string still in the table */
@@ -893,10 +933,49 @@
_eina_stringshare_small_shutdown();
eina_log_domain_unregister(_eina_stringshare_log_dom);
_eina_stringshare_log_dom = -1;
+
+ UNLOCK_BIG();
+ UNLOCK_SMALL();
+
+
return EINA_TRUE;
}
+#ifdef EFL_HAVE_PTHREAD
+
/**
+ * @internal
+ * @brief Activate the stringshare mutexs.
+ *
+ * This function activate the mutexs in the eina stringshare module. It is
called by
+ * eina_thread_init().
+ *
+ * @see eina_thread_init()
+ */
+void
+eina_stringshare_threads_init(void)
+{
+ _threads_activated = EINA_TRUE;
+}
+
+/**
+ * @internal
+ * @brief Shut down the stringshare mutexs.
+ *
+ * This function shuts down the mutexs in the stringshare module.
+ * It is called by eina_thread_shutdown().
+ *
+ * @see eina_thread_shutdown()
+ */
+void
+eina_stringshare_threads_shutdown(void)
+{
+ _threads_activated = EINA_FALSE;
+}
+
+#endif
+
+/**
* @brief Retrieve an instance of a string for use in a program.
*
* @param str The string to retrieve an instance of.
@@ -936,36 +1015,53 @@
else if (slen == 1)
return (const char *)_eina_stringshare_single + ((*str) << 1);
else if (slen < 4)
- return _eina_stringshare_small_add(str, slen);
+ {
+ LOCK_SMALL();
+ const char *s = _eina_stringshare_small_add(str, slen);
+ UNLOCK_SMALL();
+ return s;
+ }
hash = eina_hash_superfast(str, slen);
hash_num = hash & 0xFF;
hash = (hash >> 8) & EINA_STRINGSHARE_MASK;
+ LOCK_BIG();
p_bucket = share->buckets + hash_num;
+
ed = _eina_stringshare_find_hash(*p_bucket, hash);
if (!ed)
- return _eina_stringshare_add_head(p_bucket, hash, str, slen);
+ {
+ const char *s = _eina_stringshare_add_head(p_bucket, hash, str, slen);
+ UNLOCK_BIG();
+ return s;
+ }
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, NULL);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, UNLOCK_BIG(), NULL);
el = _eina_stringshare_head_find(ed, str, slen);
if (el)
{
- EINA_MAGIC_CHECK_STRINGSHARE_NODE(el);
+ EINA_MAGIC_CHECK_STRINGSHARE_NODE(el, UNLOCK_BIG());
el->references++;
+ UNLOCK_BIG();
return el->str;
}
el = _eina_stringshare_node_alloc(slen);
if (!el)
- return NULL;
+ {
+ UNLOCK_BIG();
+ return NULL;
+ }
_eina_stringshare_node_init(el, str, slen);
el->next = ed->head;
ed->head = el;
_eina_stringshare_population_head_add(ed);
+ UNLOCK_BIG();
+
return el->str;
}
@@ -1014,7 +1110,7 @@
const size_t offset = (char *)&(t.str) - (char *)&t;
node = (Eina_Stringshare_Node *)(str - offset);
- EINA_MAGIC_CHECK_STRINGSHARE_NODE(node);
+ EINA_MAGIC_CHECK_STRINGSHARE_NODE(node, );
return node;
}
@@ -1047,18 +1143,27 @@
if (slen < 2)
{
_eina_stringshare_population_add(slen);
+
return str;
}
else if (slen < 4)
{
_eina_stringshare_population_add(slen);
- return _eina_stringshare_small_add(str, slen);
+
+ LOCK_SMALL();
+ const char *s = _eina_stringshare_small_add(str, slen);
+ UNLOCK_SMALL();
+
+ return s;
}
+ LOCK_BIG();
node = _eina_stringshare_node_from_str(str);
node->references++;
DBG("str=%p (%s) refs=%u", str, str, node->references);
+ UNLOCK_BIG();
+
_eina_stringshare_population_add(node->length);
return str;
@@ -1102,15 +1207,20 @@
return;
else if (slen < 4)
{
+ LOCK_SMALL();
_eina_stringshare_small_del(str, slen);
+ UNLOCK_SMALL();
return;
}
+ LOCK_BIG();
+
node = _eina_stringshare_node_from_str(str);
if (node->references > 1)
{
node->references--;
DBG("str=%p (%s) refs=%u", str, str, node->references);
+ UNLOCK_BIG();
return;
}
@@ -1127,7 +1237,7 @@
if (!ed)
goto on_error;
- EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed);
+ EINA_MAGIC_CHECK_STRINGSHARE_HEAD(ed, UNLOCK_BIG());
if (!_eina_stringshare_head_remove_node(ed, node))
goto on_error;
@@ -1140,9 +1250,12 @@
else
_eina_stringshare_population_head_del(ed);
+ UNLOCK_BIG();
+
return;
on_error:
+ UNLOCK_BIG();
/* possible segfault happened before here, but... */
CRITICAL("EEEK trying to del non-shared stringshare \"%s\"", str);
}
@@ -1231,7 +1344,10 @@
eina_iterator_array_check(const Eina_Rbtree *rbtree __UNUSED__,
Eina_Stringshare_Head *head, struct dumpinfo *fdata)
{
Eina_Stringshare_Node *node;
-
+
+ LOCK_SMALL();
+ LOCK_BIG();
+
fdata->used += sizeof(Eina_Stringshare_Head);
for (node = head->head; node; node = node->next)
{
@@ -1243,6 +1359,10 @@
fdata->dups += node->references - 1;
fdata->unique++;
}
+
+ UNLOCK_BIG();
+ UNLOCK_SMALL();
+
return EINA_TRUE;
}
@@ -1266,7 +1386,12 @@
di.unique = 0;
printf("DDD: len ref string\n");
printf("DDD:-------------------\n");
+
+ LOCK_SMALL();
_eina_stringshare_small_dump(&di);
+ UNLOCK_SMALL();
+
+ LOCK_BIG();
for (i = 0; i < EINA_STRINGSHARE_BUCKETS; i++)
{
if (!share->buckets[i]) continue;
@@ -1294,8 +1419,11 @@
for (i = 0; i < sizeof (population_group) / sizeof (population_group[0]);
++i)
fprintf(stderr, "DDD: %i strings of length %i, max strings: %i\n",
population_group[i].count, i, population_group[i].max);
#endif
+
+ UNLOCK_BIG();
}
/**
* @}
*/
+
------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
