septag / dmon

Single header C99 portable library for monitoring filesystem changes. (Windows/Linux/MacOS)
BSD 2-Clause "Simplified" License
235 stars 19 forks source link

Unwatching dirs in the same order that they were supplied raise an assert. #26

Closed fmor closed 11 months ago

fmor commented 1 year ago

Hi,

First, thanks for sharing your work. Maybe i missed something in the documentation but dmon raise an assert when unwatching dirs in the same order that they were supplied, maybe will happen if unwatching randomly too. Below is a small test programm to illustrate this behaviour.

#define DMON_IMPL
#include "dmon.h"

#include <assert.h>

void cb( dmon_watch_id id,  dmon_action action, const char* dir, const char* filename, const char* old_filename, void* opaque )
{
}

void main()
{
    static const char* dirs[] = { "/bin/", "/home", "/tmp", "/usr", "/usr/share", "/var" };
    dmon_watch_id ids[6];
    dmon_init();

    for( int i = 0; i < 6; ++i )
    {
        ids[i] = dmon_watch( dirs[i] , cb, 0, NULL );
        assert( ids[i].id != 0 );
    }

   for( int i = 0; i < 6 ; ++i )
       dmon_unwatch( ids[i] );
   dmon_deinit();
}
krumelmonster commented 1 year ago

It seems to me that dmon.h:1202 will free space in the _dmon.watches buffer by swapping the removed element with the last element. So as soon as you unwatch ids[0], ids[5] becomes invalid as 5 is stored in 0 now. It seems a lot like a bug to me. As a workaround, I guess you could remove all three of those (windows, linux,mac) from dmon.h and you'd be fine as long as you don't create more than a total of DMON_MAX_WATCHES (64) throughout the lifetime of your program.

krumelmonster commented 1 year ago

Thinking about how to actually solve this: Is there a good reason to not just heap-allocate each dmon__watch_state individually and include a pointer to it in dmon__watch_state?

septag commented 11 months ago

sorry for a "little bit of delay" in response :D. I totally missed this thread. Anyways, I took a new approach using freelists, and it should be fixed now. Although haven't tested it that much, if there is any regression bugs, please feel free to re-open this.