www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - [Proposal] Weak reference implementation for D

reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
--- Proposal ---

The proposal is to add weak reference functionality based on 
`unstd.memory.weakref`. It can be placed e.g. in `core.memory`.

Source code: 
https://bitbucket.org/denis-sh/unstandard/src/HEAD/unstd/memory/weakref.d
Documentation: 
http://denis-sh.bitbucket.org/unstandard/unstd.memory.weakref.html
Enhancement request: http://d.puremagic.com/issues/show_bug.cgi?id=4151

--- Reasons ---

Reasons, why we do need weak reference functionality in D (Issue 4151 [1]):

   1. It has a general use.

     I suppose no question here.


   2. It's hard to implement correctly.

     As a proof here are incorrect implementations written by 
experienced developers:
       * Weak reference functionality in `std.signal` implementation:
 
https://github.com/D-Programming-Language/phobos/blob/7134b603f8c9a2e9124247ff250c9b48ea697998/std/signals.d

       * Alex's one from MCI:
 
https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d

       * Robert's one from his new `std.signals` implementation proposal:
 
https://github.com/phobos-x/phobosx/blob/d0cc6b45511465ef1d493b0d7226ccb990ae84e8/source/phobosx/signal.d

       * My implementation (fixed now, I hope):
 
https://bitbucket.org/denis-sh/unstandard/src/cb9a835a9ff5/unstd/memory/weakref.d

     Everybody can check his knowledge of concurrent programming and D 
GC by trying to understand what exactly every implementation does and 
where are race conditions. It's recommended to read implementations in 
the order provided here going to the next one as soon as you see why 
previous one is incorrect. For now the only [probably] fixed 
implementation is mine so one can see spoiler here:
 
https://bitbucket.org/denis-sh/unstandard/history-node/HEAD/unstd/memory/weakref.d
       (the first and the most fixing (spoiling you joy to understand 
everything yourself) commit is 6f59b33)


   3. It's hard to create a good API design for it.

     No jokes. E.g. there are two different behaviours of .NET weak 
references and even more in Java library.


   4. It is needed for correct signals implementation in D.

     The lack of correct signals implementation is one of [major?] 
disadvantages of D.
     Bug report: http://d.puremagic.com/issues/show_bug.cgi?id=9606

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 13 2013
next sibling parent reply Benjamin Thaut <code benjamin-thaut.de> writes:
Am 13.10.2013 09:47, schrieb Denis Shelomovskij:

 --- Proposal ---

 The proposal is to add weak reference functionality based on
 `unstd.memory.weakref`. It can be placed e.g. in `core.memory`.

 Source code:
 https://bitbucket.org/denis-sh/unstandard/src/HEAD/unstd/memory/weakref.d
 Documentation:
 http://denis-sh.bitbucket.org/unstandard/unstd.memory.weakref.html
 Enhancement request: http://d.puremagic.com/issues/show_bug.cgi?id=4151

 --- Reasons ---

 Reasons, why we do need weak reference functionality in D (Issue 4151 [1]):

    1. It has a general use.

      I suppose no question here.


    2. It's hard to implement correctly.

      As a proof here are incorrect implementations written by
 experienced developers:
        * Weak reference functionality in `std.signal` implementation:

 https://github.com/D-Programming-Language/phobos/blob/7134b603f8c9a2e9124247ff250c9b48ea697998/std/signals.d


        * Alex's one from MCI:

 https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d


        * Robert's one from his new `std.signals` implementation proposal:

 https://github.com/phobos-x/phobosx/blob/d0cc6b45511465ef1d493b0d7226ccb990ae84e8/source/phobosx/signal.d


        * My implementation (fixed now, I hope):

 https://bitbucket.org/denis-sh/unstandard/src/cb9a835a9ff5/unstd/memory/weakref.d


      Everybody can check his knowledge of concurrent programming and D
 GC by trying to understand what exactly every implementation does and
 where are race conditions. It's recommended to read implementations in
 the order provided here going to the next one as soon as you see why
 previous one is incorrect. For now the only [probably] fixed
 implementation is mine so one can see spoiler here:

 https://bitbucket.org/denis-sh/unstandard/history-node/HEAD/unstd/memory/weakref.d

        (the first and the most fixing (spoiling you joy to understand
 everything yourself) commit is 6f59b33)


    3. It's hard to create a good API design for it.

      No jokes. E.g. there are two different behaviours of .NET weak
 references and even more in Java library.


    4. It is needed for correct signals implementation in D.

      The lack of correct signals implementation is one of [major?]
 disadvantages of D.
      Bug report: http://d.puremagic.com/issues/show_bug.cgi?id=9606


I like the idea of adding weak references to phobos.

Will rt_attachDisposeEvent also work with std.allocator? Or does it rely 
on the GC running?

Kind Regards
Benjamin Thaut
Oct 13 2013
next sibling parent "Temtaime" <temtaime gmail.com> writes:
Denis, you forgot to say that it's need to download yours unstd 
library source too. Also there's only visualdproj to build it.
Oct 13 2013
prev sibling parent Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
13.10.2013 12:36, Benjamin Thaut пишет:

 Will rt_attachDisposeEvent also work with std.allocator? Or does it rely
 on the GC running?


What exactly do you mean? `rt_attachDisposeEvent` adds delegate to 
`object.__monitor.devt` array which is called from `rt_finalize2 -> 
_d_monitordelete -> _d_monitor_devt`.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 13 2013
prev sibling next sibling parent reply =?UTF-8?B?U8O2bmtlIEx1ZHdpZw==?= <sludwig outerproduct.org> writes:
Am 13.10.2013 09:47, schrieb Denis Shelomovskij:

 --- Proposal ---

 The proposal is to add weak reference functionality based on
 `unstd.memory.weakref`. It can be placed e.g. in `core.memory`.

 Source code:
 https://bitbucket.org/denis-sh/unstandard/src/HEAD/unstd/memory/weakref.d
 Documentation:
 http://denis-sh.bitbucket.org/unstandard/unstd.memory.weakref.html
 Enhancement request: http://d.puremagic.com/issues/show_bug.cgi?id=4151


+1

Bikeshed: I'd use "lock()" instead of " property target()" based on 
precedence in the form of C++'s weak_ptr, but in general that should be 
a very valuable (and long overdue) addition.

Just to reassure, the following race-condition doesn't exist, right? It 
looks like "GC.addRoot()" makes guarantees by taking the GC lock or 
something similar?

          time ->
thread1: GC collection |               | run finalizer |
thread2: paused        | lock weak ref |               | access object
Oct 13 2013
parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
13.10.2013 12:55, Sönke Ludwig пишет:

 Am 13.10.2013 09:47, schrieb Denis Shelomovskij:
 Just to reassure, the following race-condition doesn't exist, right? It
 looks like "GC.addRoot()" makes guarantees by taking the GC lock or
 something similar?

           time ->
 thread1: GC collection |               | run finalizer |
 thread2: paused        | lock weak ref |               | access object


All public GC API uses same mutex. So right, no races here. )

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 13 2013
parent reply "Michael" <pr m1xa.com> writes:
And line 61: what exactly mean a two !! in alive property?

+1 weakref
Oct 13 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 10/13/13 11:07 AM, Michael wrote:

 And line 61: what exactly mean a two !! in alive property?


"Convert this to bool".

Andrei
Oct 13 2013
parent "Michael" <pr m1xa.com> writes:
On Sunday, 13 October 2013 at 18:11:38 UTC, Andrei Alexandrescu 
wrote:

 On 10/13/13 11:07 AM, Michael wrote:

 And line 61: what exactly mean a two !! in alive property?


 "Convert this to bool".

 Andrei


Thanks)
Oct 13 2013
prev sibling next sibling parent reply Robert <jfanatiker gmx.at> writes:
        * Robert's one from his new `std.signals` implementation proposal:
  
 https://github.com/phobos-x/phobosx/blob/d0cc6b45511465ef1d493b0d7226ccb990ae84e8/source/phobosx/signal.d
 


Obviously I don't see it, otherwise I would have fixed it. Maybe you
could elaborate a bit on your claim? Your implementation uses an
entirely different technique for hiding the reference so a direct
comparison is quite hard.

Best regards,

Robert
Oct 13 2013
parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
13.10.2013 21:36, Robert пишет:

         * Robert's one from his new `std.signals` implementation proposal:

 https://github.com/phobos-x/phobosx/blob/d0cc6b45511465ef1d493b0d7226ccb990ae84e8/source/phobosx/signal.d


 Obviously I don't see it, otherwise I would have fixed it. Maybe you
 could elaborate a bit on your claim? Your implementation uses an
 entirely different technique for hiding the reference so a direct
 comparison is quite hard.


1. Have you read `gc.gc.fullcollect`, I mean a general function 
structure, not every line? If not, read it or you have no idea how 
collection performs.

2. I'm surprised you do think your implementation is correct as calling 
code twice (`foreach(i; 0..2)`) is an obvious hack to decrease a variety 
of undesired threads execution order (as it have to execute in this 
order twice).

----- Explanation -----

1. Race condition
   In every moment GC thread can be paused in state it already marked 
all dead blocks and ready to collect. So before `GC.addrOf` (which will 
have to wait for the end of the collection as it uses same mutex) call 
it can collect your object (the object can be on stack or whatever, 
doesn't matter). Also an new object can be created occupying the same 
memory and `GC.addrOf` will return non-null.

2. Incorrect assumption
   `o = GC.addrOf(tmp.address)` is just incorrect as you assume the 
object is placed at the base address of its memory block which is not 
guaranteed. Yes, it may be true for now (I haven't read GC sources 
enough to be definite here) in general case but what about some e.g. 
tricky user defined class instance sequences which user may create? Yes, 
never heard about it and just invented it, but it doesn't make this or 
similar case impossible. Also it's rather bad to do any needless 
assumption about internal stuff.

----- P.S. -----

I have to say you have a big problem especially for a programmer: you 
think you are competent in area you aren't and it can play a trick on 
you later. Please don't be angry with me, we all like to think so but we 
all have to look at ourselves as critically as possible to prevent 
problems in future.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 13 2013
parent reply "robert" <jfanatiker gmx.at> writes:
 1. Have you read `gc.gc.fullcollect`, I mean a general function 
 structure, not every line? If not, read it or you have no idea 
 how collection performs.


I haven't, I relied on: http://dlang.org/garbage.html , but I
will now - thanks. If the information at garbage.html isn't
completely wrong I have some idea though.


 2. I'm surprised you do think your implementation is correct as 
 calling code twice (`foreach(i; 0..2)`) is an obvious hack to 
 decrease a variety of undesired threads execution order (as it 
 have to execute in this order twice).


Maybe you should read the code twice ;-) In the second run I
already have a visible address to the object. I retrieve it a
second time, because exactly the GC could have kicked in right
before seeing it, but in this case my invisible address gets
reset to null so I detect this case in the second iteration
(where it can't be collected any longer). The solution might not
be perfect and there might be a better one, but in my view it
should work.


 ----- Explanation -----

 1. Race condition
   In every moment GC thread can be paused in state it already 
 marked all dead blocks and ready to collect. So before 
 `GC.addrOf` (which will have to wait for the end of the 
 collection as it uses same mutex) call it can collect your 
 object (the object can be on stack or whatever, doesn't 
 matter). Also an new object can be created occupying the same 
 memory and `GC.addrOf` will return non-null.


Exactly the reason why I retrieve it twice. This way I detect a
collection and reassignment, because my invisible address would
have been set to null. With GC.addrof I ensure that it was
already set to null before checking.


 2. Incorrect assumption
   `o = GC.addrOf(tmp.address)` is just incorrect as you assume 
 the object is placed at the base address of its memory block 
 which is not guaranteed. Yes, it may be true for now (I haven't 
 read GC sources enough to be definite here) in general case but 
 what about some e.g. tricky user defined class instance 
 sequences which user may create? Yes, never heard about it and 
 just invented it, but it doesn't make this or similar case 
 impossible. Also it's rather bad to do any needless assumption 
 about internal stuff.


That's actually a good catch, thank you. I took it from another
implementation without proper checking. Will fix it.


 ----- P.S. -----

 I have to say you have a big problem especially for a 
 programmer: you think you are competent in area you aren't and 
 it can play a trick on you later. Please don't be angry with 
 me, we all like to think so but we all have to look at 
 ourselves as critically as possible to prevent problems in 
 future.


Why would I be angry with a stranger who insults me in public? I
don't understand your concerns.

I did not write my signal implementation because of this bug, but
because of a number of other issues. I happened to stumble across
this one too, so I obviously had to fix it. If you are more
experienced in this area I am glad if you share your insights and
if you think something is wrong, I am glad to discuss it and fix
it if you are right, but just saying your implementation is
wrong, does not really help. It implies that you are obviously
right and everyone who does not see this is obviously a moron, if
someone has a bit of a problem with his ego, I don't think it is
me.
Oct 14 2013
parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
14.10.2013 13:04, robert пишет:

 Why would I be angry with a stranger who insults me in public? I
 don't understand your concerns.


No insults assumed! Just ugly truth about all of us. )


 If you are more
 experienced in this area I am glad if you share your insights and


Walter and Andrei often do silly mistakes. Can we suppose we are more 
experienced than they? Of course no, they just don't have enough time to 
think and check. Here the situation can be the same. I probably just 
have enough time to investigate the problem and solve it.


 if you think something is wrong, I am glad to discuss it and fix
 it if you are right, but just saying your implementation is
 wrong, does not really help. It implies that you are obviously
 right and everyone who does not see this is obviously a moron, if
 someone has a bit of a problem with his ego, I don't think it is
 me.


Easy, man. I have never met morons here, except, probably, myself.
Concurrent programming is fun so I just don't want to spoil the pleasure 
of investigation. And yes, I'm also lazy. )

Now about your code. First, I was completely incorrect about it, sorry 
for that. My mistake. I didn't even think the code containing such loop 
can be "so much correct". But, and this is the second, the code can't be 
"more or less" correct. It is either correct or not correct. It 
remembers me our (Russian) recent Moscow mayoral elections when we tried 
to change something in our country (we failed, yes) and after government 
won officials said: "It was the most honest elections of all preceding." )

So you code is incorrect and lets show it. When you give your code for 
eating to the compiler, it can does whatever it want but guarantee your 
program will work as you have written it (except special cases like copy 
construction elimination) and it doesn't assume every variable can be 
accessed from other threads. E.g. here is you code with unwinded loop in 
SSA (static single assignment) form:
```
auto tmp1 = atomicLoad(_obj);
void* o1 = tmp1.address;
if(o1 is null) return null;
void* o2 = GC.addrOf(tmp1.address);

auto tmp2 = atomicLoad(_obj);
void* o3 = tmp2.address;
if(o3 is null) return null;
void* o4 = GC.addrOf(tmp2.address);

if(o4) return cast(Object) o4;
return null;
```

`o1` is only used once and `o2` is never used so the compiler is free to 
discard the former and ignore the latter. So your code equals to this code:
```
auto tmp1 = atomicLoad(_obj);
if(tmp1.address is null) return null;
GC.addrOf(tmp1.address);

auto tmp2 = atomicLoad(_obj);
if(tmp2.address is null) return null;
void* o4 = GC.addrOf(tmp2.address);

if(o4) return cast(Object) o4;
return null;
```

So the second iteration gives nothing except decreasing (squaring to be 
precise) a variety of undesired threads execution order.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 14 2013
parent reply "robert" <jfanatiker gmx.at> writes:
 Easy, man. I have never met morons here, except, probably, 
 myself.


My apologies if I got you wrong!


 So you code is incorrect and lets show it. When you give your 
 code for eating to the compiler, it can does whatever it want 
 but guarantee your program will work as you have written it 
 (except special cases like copy construction elimination) and 
 it doesn't assume every variable can be accessed from other 
 threads. E.g. here is you code with unwinded loop in SSA 
 (static single assignment) form:
 ...


Damn it, you are right I did not think this through, somehow 
thought the use in addrOf is enough, which is of course crap. 
Thank's a lot for your time, I'll fix this ASAP. It is so 
obviously wrong now, it really hurts. Ouch.

Thanks again!
Oct 14 2013
parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
14.10.2013 17:42, robert пишет:

 Damn it, you are right I did not think this through, somehow thought the
 use in addrOf is enough, which is of course crap. Thank's a lot for your
 time, I'll fix this ASAP.


So, here are your revised version:
https://github.com/phobos-x/phobosx/blob/1f0016c84c2043da0b9d2dafe65f54fcf6b6b8fa/source/phobosx/signal.d

Sorry, but you are making the same mistake again.

Lets start from the hardware. Just like a compiler CPU is free to do 
whatever it wants with passed instructions but guarantee result state 
will be the same as if it is executed sequentially. And it doesn't 
assume access from other threads by default (see e.g. "out-of-order 
execution"). So memory barriers (memory fences) are needed to ensure 
loads/stores before the barrier are performed and no loads/stores after 
the barrier are executing. This is what `core.atomic.atomicFence` does 
and it can be used in e.g. in mutex implementations. As your operations 
with `_obj` are already atomic no `atomicFence` call is needed.

Now let's assume without loss of generality `InvisibleAddress.address` 
returns `cast(void*) ~_addr`, inline the `address` call, and remove 
redundant `atomicFence` call:
```
auto tmp = atomicLoad(_obj);
auto o = cast(void*) ~tmp._addr;
if(o is null) return null;
GC.addrOf(o);

auto tmp1 = atomicLoad(_obj);
if(o is cast(void*) ~tmp1._addr)
     return cast(Object) o;
assert(cast(void*) ~tmp1._addr is null);
return null;
```

As I mentioned above you are making the same incorrect assumption that 
you know what machine instructions a compiler will generate. Never make 
such assumptions. Here is an example of how your code can be rewritten 
by a compiler:
```
auto tmp = atomicLoad(_obj);
if(tmp._addr == -1) return null;
GC.addrOf(cast(void*) ~tmp._addr);

auto tmp1 = atomicLoad(_obj);
if(tmp._addr == tmp1._addr)
     return cast(Object) cast(void*) ~tmp._addr;
assert(tmp1._addr == -1);
return null;
```


-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 15 2013
next sibling parent reply "Sean Kelly" <sean invisibleduck.org> writes:
Perhaps I missed it from skimming, but why are we using atomic 
operations here anyway?  Has testing revealed that it's necessary?
Oct 15 2013
next sibling parent reply "John Colvin" <john.loughran.colvin gmail.com> writes:
On Tuesday, 15 October 2013 at 18:57:16 UTC, Sean Kelly wrote:

 Perhaps I missed it from skimming, but why are we using atomic 
 operations here anyway?  Has testing revealed that it's 
 necessary?


I presume you don't mean running some code and then seeing if it 
breaks as a test to see if atomic operation are necessary?

Synchronisation *must* be done by design.
Oct 15 2013
parent reply "Sean Kelly" <sean invisibleduck.org> writes:
On Tuesday, 15 October 2013 at 19:51:00 UTC, John Colvin wrote:

 On Tuesday, 15 October 2013 at 18:57:16 UTC, Sean Kelly wrote:

 Perhaps I missed it from skimming, but why are we using atomic 
 operations here anyway?  Has testing revealed that it's 
 necessary?


 I presume you don't mean running some code and then seeing if 
 it breaks as a test to see if atomic operation are necessary?

 Synchronisation *must* be done by design.


Well sure, but why not use a Mutex?  What does trying to sort out 
a correct lock-free algorithm gain us here?
Oct 15 2013
parent reply "Robert" <jfanatiker gmx.at> writes:
 Well sure, but why not use a Mutex?  What does trying to sort 
 out a correct lock-free algorithm gain us here?


It is not about concurrency for general purpose (phobosx.signal 
is no more thread safe than std.signals), but for the GC. A 
reference is hidden from the GC, when making it visible again you 
get a race condition with the GC. Ensuring that you really have a 
valid reference proves to be troublesome.

The problem is that destructors and thus the registered hooks for 
the dispose events are called when threads are already resumed. 
If this wasn't the case there would actually be no problems.

Best regards,

Robert
Oct 15 2013
next sibling parent "Robert" <jfanatiker gmx.at> writes:
See also: http://d.puremagic.com/issues/show_bug.cgi?id=4150

Best regards,

Robert
Oct 15 2013
prev sibling parent reply "Sean Kelly" <sean invisibleduck.org> writes:
On Tuesday, 15 October 2013 at 22:09:17 UTC, Robert wrote:

 The problem is that destructors and thus the registered hooks 
 for the dispose events are called when threads are already 
 resumed. If this wasn't the case there would actually be no 
 problems.


Gotcha.  Looking at the code... I think you'll get this to work, 
but manipulating such user-mode weak references seems really 
expensive.  Why not work on a DIP to get them built in?  For 
example, one option might be to have the GC perform certain types 
of finalization while the world is stopped.  This would have to 
be limited to very rudimentary stuff, and the easiest way to 
guarantee that would be to have everything live in Druntime.
Oct 15 2013
next sibling parent "inout" <inout gmail.com> writes:
On Tuesday, 15 October 2013 at 23:20:39 UTC, Sean Kelly wrote:

 On Tuesday, 15 October 2013 at 22:09:17 UTC, Robert wrote:

 The problem is that destructors and thus the registered hooks 
 for the dispose events are called when threads are already 
 resumed. If this wasn't the case there would actually be no 
 problems.


 Gotcha.  Looking at the code... I think you'll get this to 
 work, but manipulating such user-mode weak references seems 
 really expensive.  Why not work on a DIP to get them built in?  
 For example, one option might be to have the GC perform certain 
 types of finalization while the world is stopped.  This would 
 have to be limited to very rudimentary stuff, and the easiest 
 way to guarantee that would be to have everything live in 
 Druntime.


That makes a lot of sense, +1!
Oct 15 2013
prev sibling next sibling parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
16.10.2013 3:20, Sean Kelly пишет:

 On Tuesday, 15 October 2013 at 22:09:17 UTC, Robert wrote:

 The problem is that destructors and thus the registered hooks for the
 dispose events are called when threads are already resumed. If this
 wasn't the case there would actually be no problems.


 Gotcha.  Looking at the code... I think you'll get this to work, but
 manipulating such user-mode weak references seems really expensive.  Why
 not work on a DIP to get them built in?  For example, one option might
 be to have the GC perform certain types of finalization while the world
 is stopped.  This would have to be limited to very rudimentary stuff,
 and the easiest way to guarantee that would be to have everything live
 in Druntime.


But someone have to do it. And I can only see it will save one of two GC 
lock/unlock pairs.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 16 2013
parent "Sean Kelly" <sean invisibleduck.org> writes:
On Wednesday, 16 October 2013 at 10:02:28 UTC, Denis Shelomovskij 
wrote:

 16.10.2013 3:20, Sean Kelly пишет:
Looking at the code... I think you'll get this to work, but

 manipulating such user-mode weak references seems really 
 expensive.  Why not work on a DIP to get them built in?  For 
 example, one option might be to have the GC perform certain 
 types of finalization while the world is stopped.  This would 
 have to be limited to very rudimentary stuff, and the easiest 
 way to guarantee that would be to have everything live in 
 Druntime.


 But someone have to do it. And I can only see it will save one 
 of two GC lock/unlock pairs.


If the GC calls "block was disposed" callbacks when the world is 
stopped, it's possible that a WeakRef implementation wouldn't 
need any synchronization at all beyond whatever is necessary to 
prevent the compiler from optimizing anything away.  I haven't 
thought too hard about this though, so perhaps there's something 
I've missed.
Oct 16 2013
prev sibling parent Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
16.10.2013 3:20, Sean Kelly пишет:

 On Tuesday, 15 October 2013 at 22:09:17 UTC, Robert wrote:

 The problem is that destructors and thus the registered hooks for the
 dispose events are called when threads are already resumed. If this
 wasn't the case there would actually be no problems.


 Gotcha.  Looking at the code... I think you'll get this to work, but
 manipulating such user-mode weak references seems really expensive.  Why
 not work on a DIP to get them built in?  For example, one option might
 be to have the GC perform certain types of finalization while the world
 is stopped.  This would have to be limited to very rudimentary stuff,
 and the easiest way to guarantee that would be to have everything live
 in Druntime.


What about this:
https://github.com/D-Programming-Language/druntime/pull/639

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 16 2013
prev sibling parent Dejan Lekic <dejan.lekic gmail.com> writes:
On Tue, 15 Oct 2013 20:57:14 +0200, Sean Kelly wrote:


 Perhaps I missed it from skimming, but why are we using atomic
 operations here anyway?  Has testing revealed that it's necessary?


I believe it is the "why make it easy when we can make it complicated?" 
approach...
Oct 15 2013
prev sibling parent "Robert" <jfanatiker gmx.at> writes:
 So, here are your revised version:
 https://github.com/phobos-x/phobosx/blob/1f0016c84c2043da0b9d2dafe65f54fcf6b6b8fa/source/phobosx/signal.d

 Sorry, but you are making the same mistake again.


Yeah, I made a mistake again. In my mind it was ok because "o" is 
read from a shared variable, but this is not true, as it is read 
from a temporary, which does not hold a visible address.

It is getting embarrassing, but I really appreciate your help in 
making it a rock solid solution. Thank you! I hope my next try is 
working out better, I have to sleep over it and re-examine it, 
but maybe you want to have a look, you are usually faster and 
more reliable in finding flaws than myself ;-) :

https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d

The relevant code is now in InvisibleRef.address and 
InvisibleRef.makeVisible/makeInvisible.

Best regards,

Robert
Oct 15 2013
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 10/13/2013 12:47 AM, Denis Shelomovskij wrote:

 --- Proposal ---


Please post as a DIP:

http://wiki.dlang.org/DIPs

The trouble with it as a n.g. posting is they tend to scroll off and be
forgotten.
Oct 13 2013
parent reply Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
13.10.2013 22:19, Walter Bright пишет:

 On 10/13/2013 12:47 AM, Denis Shelomovskij wrote:

 --- Proposal ---


 Please post as a DIP:

 http://wiki.dlang.org/DIPs

 The trouble with it as a n.g. posting is they tend to scroll off and be
 forgotten.


Why? There is already enhancement request 4151 to no forget and review 
queue to add it into.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 13 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 10/13/2013 11:24 PM, Denis Shelomovskij wrote:

 13.10.2013 22:19, Walter Bright пишет:

 On 10/13/2013 12:47 AM, Denis Shelomovskij wrote:

 --- Proposal ---


 Please post as a DIP:

 http://wiki.dlang.org/DIPs

 The trouble with it as a n.g. posting is they tend to scroll off and be
 forgotten.


 Why? There is already enhancement request 4151 to no forget and review queue to
 add it into.



http://d.puremagic.com/issues/show_bug.cgi?id=4151 does not contain the info in 
your post starting this thread, nor does it contain any link to this thread.
Oct 15 2013
parent Martin Nowak <code dawg.eu> writes:
On 10/16/2013 12:45 AM, Walter Bright wrote:

 http://d.puremagic.com/issues/show_bug.cgi?id=4151 does not contain the
 info in your post starting this thread, nor does it contain any link to
 this thread.


Yeah, more cross references please.
I personally dislike the DIP proliferation for anything but big changes.
Oct 17 2013
prev sibling next sibling parent "Flamaros" <flamaros.xavier gmail.com> writes:
On Sunday, 13 October 2013 at 07:47:55 UTC, Denis Shelomovskij 
wrote:

 --- Proposal ---

 The proposal is to add weak reference functionality based on 
 `unstd.memory.weakref`. It can be placed e.g. in `core.memory`.

 Source code: 
 https://bitbucket.org/denis-sh/unstandard/src/HEAD/unstd/memory/weakref.d
 Documentation: 
 http://denis-sh.bitbucket.org/unstandard/unstd.memory.weakref.html
 Enhancement request: 
 http://d.puremagic.com/issues/show_bug.cgi?id=4151

 --- Reasons ---

 Reasons, why we do need weak reference functionality in D 
 (Issue 4151 [1]):

   1. It has a general use.

     I suppose no question here.


   2. It's hard to implement correctly.

     As a proof here are incorrect implementations written by 
 experienced developers:
       * Weak reference functionality in `std.signal` 
 implementation:

 https://github.com/D-Programming-Language/phobos/blob/7134b603f8c9a2e9124247ff250c9b48ea697998/std/signals.d

       * Alex's one from MCI:

 https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d

       * Robert's one from his new `std.signals` implementation 
 proposal:

 https://github.com/phobos-x/phobosx/blob/d0cc6b45511465ef1d493b0d7226ccb990ae84e8/source/phobosx/signal.d

       * My implementation (fixed now, I hope):

 https://bitbucket.org/denis-sh/unstandard/src/cb9a835a9ff5/unstd/memory/weakref.d

     Everybody can check his knowledge of concurrent programming 
 and D GC by trying to understand what exactly every 
 implementation does and where are race conditions. It's 
 recommended to read implementations in the order provided here 
 going to the next one as soon as you see why previous one is 
 incorrect. For now the only [probably] fixed implementation is 
 mine so one can see spoiler here:

 https://bitbucket.org/denis-sh/unstandard/history-node/HEAD/unstd/memory/weakref.d
       (the first and the most fixing (spoiling you joy to 
 understand everything yourself) commit is 6f59b33)


   3. It's hard to create a good API design for it.

     No jokes. E.g. there are two different behaviours of .NET 
 weak references and even more in Java library.


   4. It is needed for correct signals implementation in D.

     The lack of correct signals implementation is one of 
 [major?] disadvantages of D.
     Bug report: 
 http://d.puremagic.com/issues/show_bug.cgi?id=9606


+1 having weakref in phobos
Oct 13 2013
prev sibling next sibling parent "ilya-stromberg" <ilya-stromberg-2009 yandex.ru> writes:
On Sunday, 13 October 2013 at 07:47:55 UTC, Denis Shelomovskij 
wrote:

 --- Proposal ---

 The proposal is to add weak reference functionality based on 
 `unstd.memory.weakref`. It can be placed e.g. in `core.memory`.


+1
Oct 13 2013
prev sibling parent reply Martin Nowak <code dawg.eu> writes:
On 10/13/2013 09:47 AM, Denis Shelomovskij wrote:

        * Alex's one from MCI:

 https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d


I remember talking about this with Alex.
He wanted to add some functions to the GC and this is what I came up 
with based on the current implementation.
It uses the synchronized GC.addrOf to check whether the loaded pointer 
is still valid. Still looks correctly synchronized to me.
https://gist.github.com/dawgfoto/2852438

In fact the load!(msync.acq) could be made load!(msync.raw) too.
Oct 17 2013
next sibling parent Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
17.10.2013 12:09, Martin Nowak пишет:

 On 10/13/2013 09:47 AM, Denis Shelomovskij wrote:

        * Alex's one from MCI:

 https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d


 I remember talking about this with Alex.
 He wanted to add some functions to the GC and this is what I came up
 with based on the current implementation.
 It uses the synchronized GC.addrOf to check whether the loaded pointer
 is still valid. Still looks correctly synchronized to me.
 https://gist.github.com/dawgfoto/2852438

 In fact the load!(msync.acq) could be made load!(msync.raw) too.


The only thing we need from `GC.addrOf` here is a "GC barrier" i.e. 
`lock`/`unlock` pair so runtime changes are necessary for performance.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Oct 17 2013
prev sibling parent "Sean Kelly" <sean invisibleduck.org> writes:
On Thursday, 17 October 2013 at 08:09:24 UTC, Martin Nowak wrote:

 On 10/13/2013 09:47 AM, Denis Shelomovskij wrote:

       * Alex's one from MCI:

 https://github.com/lycus/mci/blob/f9165c287f92e4ef70674828fbadb33ee3967547/src/mci/core/weak.d


 I remember talking about this with Alex.
 He wanted to add some functions to the GC and this is what I 
 came up with based on the current implementation.
 It uses the synchronized GC.addrOf to check whether the loaded 
 pointer is still valid.


I'm afraid this is insufficient.  If a same-sized block is 
allocated before the dispose event is triggered, the WeakRef 
could end up pointing to something else.  It's a rare case (in 
the current GC, a finalizer would have to do the allocation), but 
possible.  This is what I referred to as the ABA problem the 
other day.  Not strictly accurate, but the effect is similar.
Oct 17 2013