Thread overview
Proper way to work around `Invalid memory operation`?
Sep 25, 2016
Matthias Klumpp
Sep 25, 2016
ketmar
Sep 26, 2016
Nemanja Boric
Sep 26, 2016
ketmar
Sep 27, 2016
Kapps
Sep 29, 2016
Kagamin
September 25, 2016
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/core/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


September 25, 2016
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); }
}
September 26, 2016
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.

September 26, 2016
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.
September 27, 2016
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/core/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.

September 29, 2016
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.