www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Proper way to work around `Invalid memory operation`?

reply Matthias Klumpp <matthias tenstral.net> writes:
Hello!

I have a class similar to this one:
```
class Dummy
{

private:

     string tmpDir;

public:

     this (string fname)
     {
         tmpDir = buildPath ("/tmp", fname.baseName);
         std.file.mkdirRecurse (tmpDir);
     }

     ~this ()
     {
         close ();
     }

     void close ()
     {
         if (std.file.exists (tmpDir))
                 std.file.rmdirRecurse (tmpDir);
     }
}
```

When the GC calls the classes destructor, I get a
`core.exception.InvalidMemoryOperationError /<...>/ldc/runtime/druntime/src/co
e/exception.d(693): Invalid memory operation`

Looks like rmdirRecurse tries to allocate with the GC, and the GC 
doesn't like that.
Is there any good way to get the temporary directory deletet 
automatically when the object is freed?
At time, I work around this bug by calling close() manually at 
the appropriate time, but this feel like a rather poor solution.

Cheers,
     Matthias
Sep 25 2016
next sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
use rc scheme (a-la std.stdio.File is using), so dtor will be 
called deterministically, not by GC. here is the sample of that, 
which creates lockfile. you can use RC implementation like that 
for many other things. it is mostly as cheap as class: the main 
struct is only size_t aka pointer (like class), and method calls 
are routed thru that pointer (like final class).

/* Written by Ketmar // Invisible Vector <ketmar ketmar.no-ip.org>
  * Understanding is not required. Only obedience.
  *
  * This program is free software: you can redistribute it and/or 
modify
  * it under the terms of the GNU General Public License as 
published by
  * the Free Software Foundation, either version 3 of the License, 
or
  * (at your option) any later version.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public 
License
  * along with this program. If not, see 
<http://www.gnu.org/licenses/>.
  */
module egflock /*is aliced*/;

import iv.egtui.types;


// 
////////////////////////////////////////////////////////////////////////// //
private struct FLockImpl {
private:
   uint rc;
   char[] fname; // malloced, 0-terminated (but 0 is not in slice)
   int fd;
   bool locked;

public nothrow  nogc:
   void kill () {
     import core.stdc.stdlib : free;
     import core.sys.posix.unistd : getpid, close;
     assert(fd >= 0);
     bool dorm = false;
     if (locked) {
       import core.sys.posix.fcntl;
       flock lk;
       lk.l_type = F_UNLCK;
       lk.l_whence = 0/*SEEK_SET*/;
       lk.l_start = 0;
       lk.l_len = 0;
       lk.l_pid = getpid();
       fcntl(fd, F_SETLK, &lk);
       locked = false;
       dorm = true;
     }
     close(fd);
     fd = -1;
     if (dorm) {
       import core.stdc.stdio : remove;
       remove(fname.ptr);
     }
     free(fname.ptr);
     fname = null;
   }

   // this will return `false` for already locked file!
   bool tryLock () {
     import core.sys.posix.fcntl;
     import core.sys.posix.unistd : getpid;
     assert(fd >= 0);
     if (locked) return false;
     flock lk;
     lk.l_type = F_WRLCK;
     lk.l_whence = 0/*SEEK_SET*/;
     lk.l_start = 0;
     lk.l_len = 0;
     lk.l_pid = getpid();
     if (fcntl(fd, F_SETLK/*W*/, &lk) == 0) locked = true;
     return locked;
   }

static:
   usize create (const(char)[] afname) {
     import core.sys.posix.fcntl /*: open*/;
     import core.sys.posix.unistd : close;
     import core.sys.posix.stdlib : malloc, free;
     if (afname.length == 0) return 0;
     auto namep = cast(char*)malloc(afname.length+1);
     if (namep is null) return 0;
     namep[0..afname.length+1] = 0;
     namep[0..afname.length] = afname[];
     auto xfd = open(namep, O_RDWR|O_CREAT/*|O_CLOEXEC*/, 
0x1b6/*0o666*/);
     if (xfd < 0) { free(namep); return 0; }
     auto fimp = cast(ubyte*)malloc(FLockImpl.sizeof);
     if (fimp is null) { close(xfd); free(namep); return 0; }
     fimp[0..FLockImpl.sizeof] = 0;
     auto res = cast(FLockImpl*)fimp;
     res.fname = namep[0..afname.length];
     res.fd = xfd;
     res.rc = 1;
     return cast(usize)fimp;
   }
}


// 
////////////////////////////////////////////////////////////////////////// //
struct FLock {
private:
   usize lci;

nothrow  trusted  nogc:
   void decref () {
     if (lci) {
       auto lcp = cast(FLockImpl*)lci;
       if (--lcp.rc == 0) lcp.kill;
       lci = 0;
     }
   }

public:
   this (const(char)[] fname) { lci = FLockImpl.create(fname); }
   ~this () { pragma(inline, true); if (lci) decref(); }
   this (this) { pragma(inline, true); if (lci) { 
++(cast(FLockImpl*)lci).rc; } }
   void opAssign (in FLock fl) {
     if (fl.lci) ++(cast(FLockImpl*)fl.lci).rc;
     decref();
     lci = fl.lci;
   }

   void close () { pragma(inline, true); if (lci != 0) decref(); }

    property bool valid () const pure { pragma(inline, true); 
return (lci != 0); }
    property bool locked () const pure { pragma(inline, true); 
return (lci != 0 ? (cast(FLockImpl*)lci).locked : false); }

   // this will return `false` for already locked file!
   bool tryLock () { pragma(inline, true); return (lci == 0 ? 
false : (cast(FLockImpl*)lci).tryLock); }
}
Sep 25 2016
prev sibling next sibling parent reply Nemanja Boric <4burgos gmail.com> writes:
On Sunday, 25 September 2016 at 16:07:12 UTC, Matthias Klumpp 
wrote:
 At time, I work around this bug by calling close() manually at 
 the appropriate time, but this feel like a rather poor solution.

 Cheers,
     Matthias
That's not a poor solution, but rather a much better solution if you rely on GC to call destructor (which it may or may not do). Consider creating few hundreds instances of a class that holds several file descriptors and then throw them away in a short time (not unusual for tests, say). It is very likely that you'll hit file descriptors limits pretty soon.
Sep 26 2016
parent ketmar <ketmar ketmar.no-ip.org> writes:
On Monday, 26 September 2016 at 09:43:02 UTC, Nemanja Boric wrote:
 On Sunday, 25 September 2016 at 16:07:12 UTC, Matthias Klumpp 
 wrote:
 At time, I work around this bug by calling close() manually at 
 the appropriate time, but this feel like a rather poor 
 solution.

 Cheers,
     Matthias
That's not a poor solution, but rather a much better solution if you rely on GC to call destructor (which it may or may not do).
yes. i want to stress that there is no *requrement* for GC to call dtors. GC which just doesn't is perfectly valid by the specs.
Sep 26 2016
prev sibling next sibling parent Kapps <opantm2+spam gmail.com> writes:
On Sunday, 25 September 2016 at 16:07:12 UTC, Matthias Klumpp 
wrote:
 Hello!

 I have a class similar to this one:
 ```
 class Dummy
 {

 private:

     string tmpDir;

 public:

     this (string fname)
     {
         tmpDir = buildPath ("/tmp", fname.baseName);
         std.file.mkdirRecurse (tmpDir);
     }

     ~this ()
     {
         close ();
     }

     void close ()
     {
         if (std.file.exists (tmpDir))
                 std.file.rmdirRecurse (tmpDir);
     }
 }
 ```

 When the GC calls the classes destructor, I get a
 `core.exception.InvalidMemoryOperationError /<...>/ldc/runtime/druntime/src/co
e/exception.d(693): Invalid memory operation`

 Looks like rmdirRecurse tries to allocate with the GC, and the 
 GC doesn't like that.
 Is there any good way to get the temporary directory deletet 
 automatically when the object is freed?
 At time, I work around this bug by calling close() manually at 
 the appropriate time, but this feel like a rather poor solution.

 Cheers,
     Matthias
As seen, you can't make GC allocations in dtors. And you should never rely on the GC calling your dtor to close things like file handles anyways. Consider making Dummy a struct and maybe using std.typecons(?).RefCounted with it.
Sep 27 2016
prev sibling parent Kagamin <spam here.lot> writes:
On Sunday, 25 September 2016 at 16:07:12 UTC, Matthias Klumpp 
wrote:
 Hello!

 I have a class similar to this one:
 ```
 class Dummy
 {

 private:

     string tmpDir;

 public:

     this (string fname)
     {
         tmpDir = buildPath ("/tmp", fname.baseName);
         std.file.mkdirRecurse (tmpDir);
     }

     ~this ()
     {
         close ();
     }

     void close ()
     {
         if (std.file.exists (tmpDir))
                 std.file.rmdirRecurse (tmpDir);
     }
 }
 ```
Another problem here is that tmpDir is GC-allocated, but by the time the constructor is called the string can be already collected, you can't read it. A possible solution would be to have a service which you would notify to delete the folder.
Sep 29 2016