D1 garbage collector + threads + malloc = garbage?

[Leandro Lucarella]

Bane, el 11 de diciembre a las 06:00 me escribiste:
> Bug with GC fullCollect() in multithreaded environment. Grauzone explained it here http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=99610
> 
> Test case that freezes app (tested on 1.053):
> 
> 
> import std.c.stdlib;
> import std.stdio;
> import std.gc;
> import std.thread;
> import std.c.time;
> 
> 
> class Foo {
>   void* p;
>   this(){
>     synchronized(this.classinfo) {
>       writefln("malloc begin");
>       p = std.c.stdlib.malloc(1024*1024);
>       writefln("malloc finished");
>     }
>   }
> 
>   ~this(){
>     synchronized(this.classinfo){
>       writefln("free begin");
>       std.c.stdlib.free(p);
>       writefln("free finished");
>     }
>   }
> }
> 
> class MyThread : Thread {
>   int run(){
>     while(true){
>       new Foo;
>       msleep(1);
>     }
>     return 0;
>   }
> }
> 
> void main(){
>   for(int i=0; i<10; i++){
>     (new MyThread).start;
>   }
>   
>   while(true){
>     writefln("collect begin");
>     std.gc.fullCollect;
>     writefln("collect finished");
>     msleep(1000);
>   }
> }
> 
> 
> 
> Can it be fixed or adleast mentioned somewhere in docs as known limitation of D1 GC? It would save a lot of time to some people (too late for me :).

The patch is very simple (as pointed by Grauzone), I guess it should make it
into D1 (I hope):

------------------------------------------------------------
diff --git a/phobos/internal/gc/gcx.d b/phobos/internal/gc/gcx.d
index b0d2821..7b1a7a3 100644
--- a/phobos/internal/gc/gcx.d
+++ b/phobos/internal/gc/gcx.d
@@ -2033,8 +2033,6 @@ struct Gcx
            }
        }
 
-       Thread.resumeAll();
-
        // Free up everything not marked
        debug(COLLECT_PRINTF) printf("\tfree'ing\n");
        size_t freedpages = 0;
@@ -2194,6 +2192,8 @@ struct Gcx
        debug(COLLECT_PRINTF) printf("recovered pages = %d\n", recoveredpages);
        debug(COLLECT_PRINTF) printf("\tfree'd %u bytes, %u pages from %u pools\
 
+       Thread.resumeAll();
+
        return freedpages + recoveredpages;
     }
 
------------------------------------------------------------

I wanted to reproduce it to make a bug report to attach the patch but
I couldn't. I replaced msleep() with usleep() (and multiplied the values
by 1000) because I'm in Linux, but that's all I changed. I have an old AMD
Athlon 1.8GHz. How do you reproduce it?

I want to be sure the bug is present before the patch and fixed after the
patch before submiting the patch via Bugzilla.

Thanks!

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Nos retiramos hasta la semana que viene reflexionando sobre nuestras
vidas: "Qué vida de mier'... Qué vida de mier'!"
	-- Sidharta Kiwi