www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Sense check: construction / deconstruction

reply Jordan Wilson <wilsonjord gmail.com> writes:
I have the following code:
import std.stdio;
import std.typecons;
import d2sqlite3;

class A {
     Database db;
     this ( Database d) {
         db = d;
     }
}

class B {
     Database* db;
     this ( Database* d) {
         db = d;
     }
}

void main() {
     auto db = Database(":memory:");
     auto a = new A(db); // gives message:
                         // Error: clean-up of Database incorrectly
                         // depends on destructors called by the GC

     auto b = new B(&db); // no message
     auto c = scoped!A(db); // no message
}

Assumption 1: "a" gives me an error message due to the fact that 
proper clean up of db depends on a being collected by the GC, and 
this behavior is being dis-allowed through use of the idiom 
https://p0nce.github.io/d-idioms/#GC-proof-resource-class?
The relevant function calling the error message is:
void ensureNotInGC(T)(string info = null) nothrow
{
     import core.exception : InvalidMemoryOperationError;
     try
     {
         import core.memory : GC;
         cast(void) GC.malloc(1);
         return;
     }
     catch(InvalidMemoryOperationError e)
     {
         // error message here
     }
}

Assumption 2: "b" gives me no error messages because the class B 
uses pointers, which moves it from relying on GC, to being 
manually free?

Assumption 3: "c" gives me no error messages because...well, I 
don't really understand why, maybe because c is in the same scope 
as db?

Thanks,

Jordan
Apr 24 2018
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 4/24/18 6:59 PM, Jordan Wilson wrote:

 I have the following code:
 import std.stdio;
 import std.typecons;
 import d2sqlite3;
 
 class A {
      Database db;
      this ( Database d) {
          db = d;
      }
 }
 
 class B {
      Database* db;
      this ( Database* d) {
          db = d;
      }
 }
 
 void main() {
      auto db = Database(":memory:");
      auto a = new A(db); // gives message:
                          // Error: clean-up of Database
incorrectly
                          // depends on destructors
called by the GC
 
      auto b = new B(&db); // no message
      auto c = scoped!A(db); // no message
 }
 
 Assumption 1: "a" gives me an error message due to the fact that proper 
 clean up of db depends on a being collected by the GC, and this behavior 
 is being dis-allowed through use of the idiom 
 https://p0nce.github.io/d-idioms/#GC-proof-resource-class?
 The relevant function calling the error message is:
 void ensureNotInGC(T)(string info = null) nothrow
 {
      import core.exception : InvalidMemoryOperationError;
      try
      {
          import core.memory : GC;
          cast(void) GC.malloc(1);
          return;
      }
      catch(InvalidMemoryOperationError e)
      {
          // error message here
      }
 }
 
 Assumption 2: "b" gives me no error messages because the class B uses 
 pointers, which moves it from relying on GC, to being manually free?
 
 Assumption 3: "c" gives me no error messages because...well, I don't 
 really understand why, maybe because c is in the same scope as db?


What you are missing is that Database is pass-by-value, not a class. So 
when you include it directly in a class like you did in A, then when A's 
destructor is called, db's destructor is called.

Since in the first case, a is being destroyed by the GC, you get the error.

In the second case (b), you aren't including the db by value, so no 
destructor is called from the GC. But this is dangerous, because db 
stops existing after main exits, but b continues to exist in the GC, so 
this is a dangling pointer.

In the third case, scoped specifically destroys c when main exits, and 
you are not in the GC at that point.

What the error message is telling you is you should manually clean up 
the database directly instead of leaving it to the GC. What is the 
correct path? probably the scoped!A version. Though I'm not sure what 
making copies of the database does in that library.

-Steve
Apr 24 2018
parent reply Jordan Wilson <wilsonjord gmail.com> writes:
On Tuesday, 24 April 2018 at 23:49:14 UTC, Steven Schveighoffer 
wrote:

 What you are missing is that Database is pass-by-value, not a 
 class. So when you include it directly in a class like you did 
 in A, then when A's destructor is called, db's destructor is 
 called.

 Since in the first case, a is being destroyed by the GC, you 
 get the error.


Ok, this makes sense.


 In the second case (b), you aren't including the db by value, 
 so no destructor is called from the GC. But this is dangerous, 
 because db stops existing after main exits, but b continues to 
 exist in the GC, so this is a dangling pointer.


If I set the pointer to null, before (b) is collected, would that 
work?


 In the third case, scoped specifically destroys c when main 
 exits, and you are not in the GC at that point.

 What the error message is telling you is you should manually 
 clean up the database directly instead of leaving it to the GC. 
 What is the correct path? probably the scoped!A version. Though 
 I'm not sure what making copies of the database does in that 
 library.

 -Steve


Ok I'll need to read the docs on scoped I think, but I think I 
understand.

If I wanted db to be persistent, but have temporary objects 
reference db without triggering GC collection of the db, you 
would use scoped?
Or is this a situation where it's better to pass the db in 
function calls to objects rather than set as a member of these 
objects?
Apr 25 2018
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 4/25/18 5:51 AM, Jordan Wilson wrote:

 On Tuesday, 24 April 2018 at 23:49:14 UTC, Steven Schveighoffer wrote:

 In the second case (b), you aren't including the db by value, so no 
 destructor is called from the GC. But this is dangerous, because db 
 stops existing after main exits, but b continues to exist in the GC, 
 so this is a dangling pointer.


 
 If I set the pointer to null, before (b) is collected, would that work?


More importantly, set it to null before Database goes out of scope.

This is what I'd recommend (if you want to go this path):

auto b = new B(&db);
scope(exit) b.db = null;

But I don't know why you wouldn't just use scoped.


 In the third case, scoped specifically destroys c when main exits, and 
 you are not in the GC at that point.

 What the error message is telling you is you should manually clean up 
 the database directly instead of leaving it to the GC. What is the 
 correct path? probably the scoped!A version. Though I'm not sure what 
 making copies of the database does in that library.


 
 Ok I'll need to read the docs on scoped I think, but I think I understand.
 
 If I wanted db to be persistent, but have temporary objects reference db 
 without triggering GC collection of the db, you would use scoped?
 Or is this a situation where it's better to pass the db in function 
 calls to objects rather than set as a member of these objects?


In that case, using a pointer is OK, as long as you put the DB in thread 
local storage (so it doesn't go out of scope).

Just put it at module level somewhere. In my projects, I have a pool of 
connections that is used, and the pool is in TLS. When I want a 
connection, I just grab the next one available, and it's reference 
counted, so it's released back to the pool automatically whenever 
anything goes out of scope. However, all my types are structs, and I 
don't put them on the GC.

-Steve
Apr 25 2018
parent Jordan Wilson <wilsonjord gmail.com> writes:
On Wednesday, 25 April 2018 at 13:52:16 UTC, Steven Schveighoffer 
wrote:

 [...]


Great, thanks for you help Steve, I'll have a think about how I 
want to structure things.

Jordan
Apr 25 2018