Re: [PATCH] client: Detach threads since they are never joined

[Slava Monich]

On 01/09/14 16:23, Patrik Flykt wrote:

Hi,

On Mon, 2014-09-01 at 15:52 +0300, Slava Monich wrote:

On 01/09/14 13:16, Patrik Flykt wrote:

On Sat, 2014-08-30 at 15:15 +0300, Slava Monich wrote:

This is for pacrunner.

More motivation in here why this patch is good, please.

Cheers,

Patrik


 A thread may either be/joinable/  or/detached/.  If a thread is
 joinable, then another thread can callpthread_join(3)  
  to wait for
 the thread to terminate and fetch its exit status.  Only when a
 terminated joinable thread has been joined are the last of its
 resources released back to the system.  When a detached thread
 terminates, its resources are automatically released back to the
 system: it is not possible to join with the thread in order to obtain
 its exit status.  Making a thread detached is useful for some types
 of daemon threads whose exit status the application does not need to
 care about.  By default, a new thread is created in a joinable state,
 unless/attr/  was set to create the thread in a detached state (using
 pthread_attr_setdetachstate(3)  
).

No, I'm not looking for a man page here. A proper commit message on what
is fixed will do fine.

The subject line is fine summary, but "This is for pacrunner" is not a
proper commit message. I was looking for the body of the commit message
saying something like "Unless the threads are detached,  will be
lost on ". Or something suitable. So that when the next person looks
at the changed lines and checks the individual commit, he goes: "Ah, of
course!"



Of course threads that are never joined have to be detached. I really 
don't know what to add. I thought (and still think) that the subject 
line says it all.


"This is for pacrunner" just means "sorry I couldn't find a dedicated 
mailing list for pacrunner but I hope this patch will reach the right 
people if I send it to connman@connman.net".


Regards,
-Slava
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] client: Detach threads since they are never joined

[Patrik Flykt]

Hi,

On Mon, 2014-09-01 at 15:52 +0300, Slava Monich wrote:
> On 01/09/14 13:16, Patrik Flykt wrote:
> > On Sat, 2014-08-30 at 15:15 +0300, Slava Monich wrote:
> >> This is for pacrunner.
> > More motivation in here why this patch is good, please.
> >
> > Cheers,
> >
> > Patrik
> >
> 
> A thread may either be/joinable/  or/detached/.  If a thread is
> joinable, then another thread can callpthread_join(3)  
>   to wait for
> the thread to terminate and fetch its exit status.  Only when a
> terminated joinable thread has been joined are the last of its
> resources released back to the system.  When a detached thread
> terminates, its resources are automatically released back to the
> system: it is not possible to join with the thread in order to obtain
> its exit status.  Making a thread detached is useful for some types
> of daemon threads whose exit status the application does not need to
> care about.  By default, a new thread is created in a joinable state,
> unless/attr/  was set to create the thread in a detached state (using
> pthread_attr_setdetachstate(3)  
> ).

No, I'm not looking for a man page here. A proper commit message on what
is fixed will do fine.

The subject line is fine summary, but "This is for pacrunner" is not a
proper commit message. I was looking for the body of the commit message
saying something like "Unless the threads are detached,  will be
lost on ". Or something suitable. So that when the next person looks
at the changed lines and checks the individual commit, he goes: "Ah, of
course!"


Cheers,

Patrik


> And here is a piece of evidence from valgrind:
> 
> ==14392== HEAP SUMMARY:
> ==14392== in use at exit: 99,659 bytes in 3,563 blocks
> ==14392== total heap usage: 11,478 allocs, 7,915 frees, 8,096,821 bytes 
> allocated
> ==14392==
> ==14392== 31,104 bytes in 216 blocks are possibly lost in loss record 
> 709 of 709
> ==14392== at 0x483643C: calloc (vg_replace_malloc.c:593)
> ==14392== by 0x4013B37: _dl_allocate_tls (dl-tls.c:297)
> ==14392== by 0x49C0947: pthread_create@@GLIBC_2.4 (allocatestack.c:571)
> ==14392== by 0x1418F: find_proxy_for_url (client.c:97)
> ==14392== by 0x107CB: process_message (object.c:259)
> ==14392== by 0x49F58A3: _dbus_object_tree_dispatch_and_unlock 
> (dbus-object-tree.c:862)
> ==14392== by 0x49E5F9B: dbus_connection_dispatch (dbus-connection.c:4672)
> ==14392== by 0xD66B: message_dispatch (mainloop.c:72)
> ==14392== by 0x48F7A8B: g_idle_dispatch (gmain.c:5251)
> ==14392== by 0x48FBB1F: g_main_context_dispatch (gmain.c:3066)
> ==14392== by 0x48FBE23: g_main_context_iterate.part.19 (gmain.c:3713)
> ==14392== by 0x48FC48B: g_main_loop_run (gmain.c:3906)
> ==14392==
> ==14392== LEAK SUMMARY:
> ==14392== definitely lost: 0 bytes in 0 blocks
> ==14392== indirectly lost: 0 bytes in 0 blocks
> ==14392== possibly lost: 31,104 bytes in 216 blocks
> ==14392== still reachable: 68,555 bytes in 3,347 blocks
> ==14392== suppressed: 0 bytes in 0 blocks
> 
> And I'm pretty sure it was leaking some kernel resources as well.
> 
> Regards,
> -Slava
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] client: Detach threads since they are never joined

[Slava Monich]

On 01/09/14 13:16, Patrik Flykt wrote:

On Sat, 2014-08-30 at 15:15 +0300, Slava Monich wrote:

This is for pacrunner.

More motivation in here why this patch is good, please.

Cheers,

Patrik



   A thread may either be/joinable/  or/detached/.  If a thread is
   joinable, then another thread can callpthread_join(3)  
  to wait for
   the thread to terminate and fetch its exit status.  Only when a
   terminated joinable thread has been joined are the last of its
   resources released back to the system.  When a detached thread
   terminates, its resources are automatically released back to the
   system: it is not possible to join with the thread in order to obtain
   its exit status.  Making a thread detached is useful for some types
   of daemon threads whose exit status the application does not need to
   care about.  By default, a new thread is created in a joinable state,
   unless/attr/  was set to create the thread in a detached state (using
   pthread_attr_setdetachstate(3)  
).


And here is a piece of evidence from valgrind:

==14392== HEAP SUMMARY:
==14392== in use at exit: 99,659 bytes in 3,563 blocks
==14392== total heap usage: 11,478 allocs, 7,915 frees, 8,096,821 bytes 
allocated

==14392==
==14392== 31,104 bytes in 216 blocks are possibly lost in loss record 
709 of 709

==14392== at 0x483643C: calloc (vg_replace_malloc.c:593)
==14392== by 0x4013B37: _dl_allocate_tls (dl-tls.c:297)
==14392== by 0x49C0947: pthread_create@@GLIBC_2.4 (allocatestack.c:571)
==14392== by 0x1418F: find_proxy_for_url (client.c:97)
==14392== by 0x107CB: process_message (object.c:259)
==14392== by 0x49F58A3: _dbus_object_tree_dispatch_and_unlock 
(dbus-object-tree.c:862)

==14392== by 0x49E5F9B: dbus_connection_dispatch (dbus-connection.c:4672)
==14392== by 0xD66B: message_dispatch (mainloop.c:72)
==14392== by 0x48F7A8B: g_idle_dispatch (gmain.c:5251)
==14392== by 0x48FBB1F: g_main_context_dispatch (gmain.c:3066)
==14392== by 0x48FBE23: g_main_context_iterate.part.19 (gmain.c:3713)
==14392== by 0x48FC48B: g_main_loop_run (gmain.c:3906)
==14392==
==14392== LEAK SUMMARY:
==14392== definitely lost: 0 bytes in 0 blocks
==14392== indirectly lost: 0 bytes in 0 blocks
==14392== possibly lost: 31,104 bytes in 216 blocks
==14392== still reachable: 68,555 bytes in 3,347 blocks
==14392== suppressed: 0 bytes in 0 blocks

And I'm pretty sure it was leaking some kernel resources as well.

Regards,
-Slava
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] client: Detach threads since they are never joined

[Patrik Flykt]

On Sat, 2014-08-30 at 15:15 +0300, Slava Monich wrote:
> This is for pacrunner.

More motivation in here why this patch is good, please.

Cheers,

Patrik




___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] client: Detach threads since they are never joined

[Tomasz Bursztyka]

Hi,


This is for pacrunner.


If a patch is for pacrunner, specify it in the subject of the patch 
[PATCH pacrunnre] ...

Commit message should have patch explanations only.

Thanks,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] client: Detach threads since they are never joined

[Slava Monich]

This is for pacrunner.

---
 src/client.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/client.c b/src/client.c
index d354c2b..8d1e22b 100644
--- a/src/client.c
+++ b/src/client.c
@@ -84,6 +84,7 @@ static DBusMessage *find_proxy_for_url(DBusConnection *conn,
DBusMessage *msg, void *user_data)
 {
struct jsrun_data *jsrun;
+   pthread_attr_t attrs;
 
jsrun = g_try_new0(struct jsrun_data, 1);
if (!jsrun)
@@ -94,7 +95,9 @@ static DBusMessage *find_proxy_for_url(DBusConnection *conn,
jsrun->conn = dbus_connection_ref(conn);
jsrun->msg = dbus_message_ref(msg);
 
-   if (pthread_create(&jsrun->thread, NULL, jsrun_thread, jsrun) != 0) {
+   pthread_attr_init(&attrs);
+   pthread_attr_setdetachstate(&attrs, PTHREAD_CREATE_DETACHED);
+   if (pthread_create(&jsrun->thread, &attrs, jsrun_thread, jsrun) != 0) {
jsrun_free(jsrun);
return g_dbus_create_error(msg,
PACRUNNER_ERROR_INTERFACE ".Failed",
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman