Jump to page: 1 2
Thread overview
unique_ptr | Unique for autoclose handle
Dec 14, 2022
Vitaliy Fadeev
Dec 14, 2022
Sergey
Dec 14, 2022
Leonardo
Dec 14, 2022
Vitaliy Fadeev
Dec 14, 2022
Ali Çehreli
Dec 14, 2022
Ali Çehreli
Dec 15, 2022
Vitaliy Fadeev
Dec 16, 2022
Vitaliy Fadeev
Dec 15, 2022
Vitaliy Fadeev
Unique!struct bug - Re: unique_ptr | Unique for autoclose handle
Dec 15, 2022
Nick Treleaven
Dec 15, 2022
Ali Çehreli
Dec 16, 2022
Nick Treleaven
December 14, 2022

Hi! I open a device under Windows:

HANDLE h = CreateFileW( ... );

in procedure:

HANDLE open_keyboard_device2( LPCWSTR path, int* error_number )
{
   ...

    HANDLE dev_handle =
            CreateFileW(
                path,
                0,
                FILE_SHARE_READ | FILE_SHARE_WRITE,
                NULL,
                OPEN_EXISTING,
                0,
                NULL
            );

    ...

    return dev_handle;
}

and I want to close HANDLE automatically, when dev_handle destroyed by GC:

CloseHandle( h );

How to do it?
How to define HANDLE var ? What to return from procedure? How to call CloseHandle( h ) when variable destroyed?

I was trying std.typecons.Unique. But where I must put CloseHandle( h ) ?
I was trying std.typecons.Unique with custom class SafeHabdle

class SafeHandle
{
    HANDLE h;

    this( HANDLE h )
    {
        this.h = h;
    }

    ~this()
    {
        if ( h != INVALID_HANDLE_VALUE )
            CloseHandle( h );
    }
}

and using it:

Unique!SafeHandle open_keyboard_device2( LPCWSTR path, int* error_number )
{
    ...
    Unique!SafeHandle dev_handle =
        new SafeHandle(
            CreateFileW(
                path,
                0,
                FILE_SHARE_READ | FILE_SHARE_WRITE,
                NULL,
                OPEN_EXISTING,
                0,
                NULL
            )
        );
    ...
}

It complex. Because needed:

    Unique!SafeHandle open_keyboard_device2( LPCWSTR path, int* error_number )
    Unique!SafeHandle dev_handle = new SafeHandle( CreateFileW( ... ) );
    DeviceIoControl( dev_handle.h, ...);

vs

    HANDLE open_keyboard_device2( LPCWSTR path, int* error_number )
    HANDLE dev_handle = CreateFileW( ... );
    DeviceIoControl( dev_handle, ...);

Last is readable.

Teach me the most beautiful way.
How to make beautiful?
Thanks!

December 14, 2022

On Wednesday, 14 December 2022 at 11:30:07 UTC, Vitaliy Fadeev wrote:

>

Teach me the most beautiful way.
How to make beautiful?
Thanks!

Just for information there is a library that also could be helpful https://code.dlang.org/packages/autoptr

December 14, 2022

On Wednesday, 14 December 2022 at 11:30:07 UTC, Vitaliy Fadeev wrote:

>

Hi! I open a device under Windows:

HANDLE h = CreateFileW( ... );

in procedure:

HANDLE open_keyboard_device2( LPCWSTR path, int* error_number )
{
   ...

    HANDLE dev_handle =
            CreateFileW(
                path,
                0,
                FILE_SHARE_READ | FILE_SHARE_WRITE,
                NULL,
                OPEN_EXISTING,
                0,
                NULL
            );

    ...

    return dev_handle;
}

and I want to close HANDLE automatically, when dev_handle destroyed by GC:

CloseHandle( h );

How to do it?
How to define HANDLE var ? What to return from procedure? How to call CloseHandle( h ) when variable destroyed?

I was trying std.typecons.Unique. But where I must put CloseHandle( h ) ?
I was trying std.typecons.Unique with custom class SafeHabdle

class SafeHandle
{
    HANDLE h;

    this( HANDLE h )
    {
        this.h = h;
    }

    ~this()
    {
        if ( h != INVALID_HANDLE_VALUE )
            CloseHandle( h );
    }
}

and using it:

Unique!SafeHandle open_keyboard_device2( LPCWSTR path, int* error_number )
{
    ...
    Unique!SafeHandle dev_handle =
        new SafeHandle(
            CreateFileW(
                path,
                0,
                FILE_SHARE_READ | FILE_SHARE_WRITE,
                NULL,
                OPEN_EXISTING,
                0,
                NULL
            )
        );
    ...
}

It complex. Because needed:

    Unique!SafeHandle open_keyboard_device2( LPCWSTR path, int* error_number )
    Unique!SafeHandle dev_handle = new SafeHandle( CreateFileW( ... ) );
    DeviceIoControl( dev_handle.h, ...);

vs

    HANDLE open_keyboard_device2( LPCWSTR path, int* error_number )
    HANDLE dev_handle = CreateFileW( ... );
    DeviceIoControl( dev_handle, ...);

Last is readable.

Teach me the most beautiful way.
How to make beautiful?
Thanks!

If you need an specific shutdown maybe you can use scopes. But I'm here learning too.
https://tour.dlang.org/tour/en/gems/scope-guards

December 14, 2022

On Wednesday, 14 December 2022 at 11:30:07 UTC, Vitaliy Fadeev wrote:

>

How to define HANDLE var ? What to return from procedure? How to call CloseHandle( h ) when variable destroyed?

I was trying std.typecons.Unique. But where I must put CloseHandle( h ) ?
I was trying std.typecons.Unique with custom class SafeHabdle

Last try is:

struct SafeHandle
{
    Unique!void _safe;
    alias _safe this;

    this( HANDLE h )
    {
        this._safe = h;
    }

    ~this()
    {
        if ( cast(HANDLE)_safe !is null )
        if ( cast(HANDLE)_safe != INVALID_HANDLE_VALUE )
        {
            if ( CloseHandle( cast(HANDLE)_safe ) == 0 )
                cast(HANDLE)_safe = null;
        }
    }

    ref HANDLE get()
    {
        return cast( HANDLE )_safe;
    }
}

and using:

SafeHandle open_keyboard_device2( LPCWSTR path, int* error_number )
{
   ...
   SafeHandle dev_handle =
        CreateFileW(
            path,
            0,
            FILE_SHARE_READ | FILE_SHARE_WRITE,
            NULL,
            OPEN_EXISTING,
            0,
            NULL
        );

    if ( dev_handle.get() != INVALID_HANDLE_VALUE )
        ...
   ...
}

void processDevice( ... )
{
    auto dev_handle = open_keyboard_device2( path, &err );
    set_keyboard_indicator2( dev_handle, KEYBOARD_CAPS_LOCK_ON );
    ...
}

May be good, but testing...

December 14, 2022
On 12/14/22 05:58, Vitaliy Fadeev wrote:
> On Wednesday, 14 December 2022 at 11:30:07 UTC, Vitaliy Fadeev wrote:
>> How to define HANDLE var ?  What to return from procedure? How to call
>> CloseHandle( h ) when variable destroyed?

An obvious way is an RAII type where the destructor calls CloseHandle.

> struct SafeHandle
> {
>      Unique!void _safe;

So you made Unique a member of SafeHandle. I've never used Unique but I think it has a bug (or a design issue?): Its destructor is the following:

    ~this()
    {
        if (_p !is null)
        {
            destroy(_p);
            _p = null;
        }
    }

Because _p is a pointer, destroy(_p) will not dereference and destroy what it points to. I think this is a bug with Unique. I think it should do

  destroy(*_p);

In any case, I would use a Handle RAII type that calls CloseHandle in its destructor. Here is the code that made sense to me:

import std;

// Some values and types to make the code compile:
alias HANDLE = void*;
alias LPCWSTR = string;
enum INVALID_HANDLE_VALUE = null;
enum FILE_SHARE_READ = 1;
enum FILE_SHARE_WRITE = 2;
enum NULL = null;
enum OPEN_EXISTING = 1000;

// Some mocks of system functions
HANDLE CreateFileW(LPCWSTR path, int, int, void*, int, int, void*) {
    auto handle = cast(HANDLE)(new int(42));
    writeln("Created ", handle);
    return handle;
}

int CloseHandle(HANDLE handle) {
    writeln("Closing ", handle);
    return 0;
}

// This is the RAII type for closing system handles
struct Handle {
    HANDLE value;

    // Disabling copying and assignment
    @disable this(this);
    @disable typeof(this) opAssign(const(typeof(this)));

    this(HANDLE value) {
        this.value = value;
        writeln("Constructed Handle with ", value);
    }

    ~this() {
        const ret = CloseHandle(value);
        if (ret) {
            stderr.writefln!"Failed to close handle %s"(value);
        }
    }
}

Handle open_keyboard_device2( LPCWSTR path, int* error_number )
{
   // ...
   HANDLE dev_handle =
        CreateFileW(
            path,
            0,
            FILE_SHARE_READ | FILE_SHARE_WRITE,
            NULL,
            OPEN_EXISTING,
            0,
            NULL
        );

   // According to documentation, the handler must be created dynamically:
   // We make a unique owner for it:
   auto result = Handle(dev_handle);
   writeln("Exiting open_keyboard_device2");
   return result;

    // if ( dev_handle.get() != INVALID_HANDLE_VALUE ) {
    //     // ...
    // }
   // ...
}

void processDevice( ... )
{
    int err;
    auto dev_handle = open_keyboard_device2("foo", &err );
    // set_keyboard_indicator2( dev_handle, KEYBOARD_CAPS_LOCK_ON );
    // ...
    writeln("Exiting processDevice");
}

void main() {
    processDevice();
    writeln("Exiting main");
}

The output of the program looks acceptable to me:

Created 7F1BD5A7D000
Constructed Handle with 7F1BD5A7D000
Exiting open_keyboard_device2
Exiting processDevice
Closing 7F1BD5A7D000
Exiting main

Ali

December 14, 2022
On 12/14/22 09:41, Ali Çehreli wrote:

>     // According to documentation, the handler must be created dynamically:
>     // We make a unique owner for it:

Ignore that part. It's a leftover from my experiments with Unique!Handle.

Ali

December 15, 2022
On Wednesday, 14 December 2022 at 17:41:07 UTC, Ali Çehreli wrote:
> On 12/14/22 05:58, Vitaliy Fadeev wrote:
> > On Wednesday, 14 December 2022 at 11:30:07 UTC, Vitaliy
> Fadeev wrote:
> >> How to define HANDLE var ?  What to return from procedure?
> How to call
> >> CloseHandle( h ) when variable destroyed?
>
> An obvious way is an RAII type where the destructor calls CloseHandle.
>
> ...
>
> Created 7F1BD5A7D000
> Constructed Handle with 7F1BD5A7D000
> Exiting open_keyboard_device2
> Exiting processDevice
> Closing 7F1BD5A7D000
> Exiting main
>
> Ali

You like a boss, Ali. Thank, Ali!

December 15, 2022

On Wednesday, 14 December 2022 at 17:44:05 UTC, Ali Çehreli wrote:

>

On 12/14/22 09:41, Ali Çehreli wrote:

>
// According to documentation, the handler must be

created dynamically:

>
// We make a unique owner for it:

Last try is customized std.typecons.Unique:

module safehandle;

import core.sys.windows.windows;
import std.typecons;

struct Unique( T, alias DTOR )
{
    ...

    ~this()
    {
        if (_p !is null)
        {
            DTOR( _p );
            destroy(_p);
            _p = null;
        }
    }

   ...
}

alias SafeHandle = Unique!(void,CloseHandle);

and using :

SafeHandle open_keyboard_device2( ... )
{
    ...

    SafeHandle dev_handle =
        CreateFileW( ... );

    ...

    return dev_handle;
}
void main()
{
    ...

    auto dev = open_keyboard_device2( ... );
    process_device( dev );

    ...

    // auto-close handle
}

Open source is good!
Thanks all!

and in future:

alias SafeHandle        = Unique!(void,CloseHandle)
alias Safe_SDL_Window   = Unique!(SDL_Window,SDL_DestroyWindow)
alias Safe_SDL_Surface  = Unique!(SDL_Surface,SDL_FreeSurface)
alias Safe_SDL_Texture  = Unique!(SDL_Texture,SDL_DestroyTexture)
alias Safe_SDL_Renderer = Unique!(SDL_Renderer,SDL_DestroyRenderer)

Code is good, but testing....

December 15, 2022
On Wednesday, 14 December 2022 at 17:41:07 UTC, Ali Çehreli wrote:
> I've never used Unique but I think it has a bug (or a design issue?): Its destructor is the following:
>
>     ~this()
>     {
>         if (_p !is null)
>         {
>             destroy(_p);
>             _p = null;
>         }
>     }
>
> Because _p is a pointer, destroy(_p) will not dereference and destroy what it points to. I think this is a bug with Unique. I think it should do
>
>   destroy(*_p);

Now filed:
https://issues.dlang.org/show_bug.cgi?id=23561

Do you think it's OK to just fix this or do we need to do some kind of deprecation?
December 15, 2022
On 12/15/22 11:31, Nick Treleaven wrote:
> On Wednesday, 14 December 2022 at 17:41:07 UTC, Ali Çehreli wrote:
>> I've never used Unique but I think it has a bug (or a design issue?):
>> Its destructor is the following:
>>
>>     ~this()
>>     {
>>         if (_p !is null)
>>         {
>>             destroy(_p);
>>             _p = null;
>>         }
>>     }
>>
>> Because _p is a pointer, destroy(_p) will not dereference and destroy
>> what it points to. I think this is a bug with Unique. I think it
>> should do
>>
>>   destroy(*_p);
>
> Now filed:
> https://issues.dlang.org/show_bug.cgi?id=23561

Thanks. I was hoping others more experienced with Phobos implementation chime in. But to me, the intention is to destroy the object. One never wants to destroy a pointer as there is no operation there.

As a minor proud moment, I do cover this issue:

  http://ddili.org/ders/d.en/memory.html#ix_memory.destroy

> Do you think it's OK to just fix this or

I think this is a bug because the documentation clearly talks about destroying the object:

  https://dlang.org/library/std/typecons/unique.html

"When a Unique!T goes out of scope it will call destroy on the
resource T that it manages, unless it is transferred. One
important consequence of destroy is that it will call the
destructor of the resource T."

>  do we need to do some kind of deprecation?

The behavior is so different from the intention that I don't think anybody is using Unique anyway. :o)

Ali

« First   ‹ Prev
1 2