0

I'm trying to implement a lockless linked list in C and having it such that you can instantiate multiple instances of the lockless linked list using the create

The code is the following:

typedef struct node_s node_t;
struct node_s {
    node_t *next;
    void *data;
    uint32_t tag;
};
typedef struct list_s list;
struct list_s
{
    node_t *head;
    node_t *tail;
};

list *create_queue(void){
    list* _list =NULL;
    _list = (list*)malloc(sizeof(list));
    node_t *head = NULL;
    node_t *tail = NULL;
    head = (node_t*)malloc(sizeof(node_t));
    if (!head) {
        fprintf(stderr, "malloc error\n");
        return NULL;
    }
    head->next = NULL;
    head->data = NULL;
    tail = head;
    _list->head = head;
    _list->tail = head;
    return _list;
}



node_t* enqueue(void *d, list *_list)
{
    if (_list == NULL)
    {
        fprintf(stderr, "List error\n");
        return NULL;
    }
    node_t *p = (node_t*)malloc(sizeof(node_t));
    if (!p) {
        fprintf(stderr, "malloc error\n");
        return NULL;
    } 
    p->next = NULL;
    p->data = d;
    node_t *q;
    do {
        q =  &_list->tail;
    } while (!__sync_bool_compare_and_swap(&(q->next), NULL, p));
    __sync_bool_compare_and_swap( &(_list->tail), q, p);
    return p;
}



void* dequeue(list *_list)
{
    node_t *p;
    void *res;
    if (_list ==NULL)
    {
        fprintf(stderr, "List error\n");
        return NULL;
    }

    do {
        p = _list->head;
        if (p->next == NULL){
            return NULL;
        }
         res = p->next->data;
    } while(!__sync_bool_compare_and_swap(&_list->head, p, p->next));
    if (p) {
        free(p);
        p = NULL;
    }
    return res;
}

The issue is that it is giving a segmentation fault when I try to use enqueue and then subsequently dequeue. So if you can point out the issue that would be of great help.

Any help would be appreciated.

coder
  • 119
  • 1
  • 2
  • 10
  • `q = &_list->tail;` --> `q = _list->tail;`. `q` is a `node_t*` type and `_list->tail` is a `node_t*` type,, `q = &_list->tail;` tries to assign a `node_t**` type to a `node_t*` type. I would expect a compiler warning from this. – yano Jun 14 '18 at 18:22
  • 1
    Not really an answer to your question, but it looks like you're using GCC's atomic builtins. Since C11, the language has official atomic support without needing to use compiler extensions. http://en.cppreference.com/w/c/atomic – Christian Gibbons Jun 14 '18 at 18:23
  • If I do the q = _list->tail;. It fails at the while loop with seg fault. What can the issue be – coder Jun 14 '18 at 18:27
  • perhaps you need a `node_t**` type there, in which case change `node_t* q;` to `node_t** q;`. The types have to match whatever assignment you need to make there. Dunno what the `__sync_bool_compare_and_swap` function is doing, an MCVE would be helpful. – yano Jun 14 '18 at 18:31
  • If I do node_t** then the __sync_bool_compare_and_swap function doesn't work since it has a different signature. – coder Jun 14 '18 at 18:41
  • So once issue I find is that the tail is null which I find quite odd. – coder Jun 14 '18 at 18:47
  • @coder `q = _list->tail;` should be right. If you try to `q->next` with it as a `node_t**` it should not compile, as you are only dereferencing it once with the arrow. – Robert Derber Jun 14 '18 at 22:13
  • 1
    I am not clear what the point of the tail pointer actually is. This is a singly linked list yes? You should only have the head pointer, the list data structure is just a pointer to data and the next element in the list, if any. There's no tail in a list. You also can't do more than one atomic operation on a lock-free data structure unless every one of them is independent of the others. In other words, trying to do two things in a row, using independent atomic operations does not get both done atomically. – jwdonahue Jun 14 '18 at 23:24
  • @jwdonahue this list has a tail pointer because it is a queue, with insertions at the back. You can have more than one atomic operations on a lock-free data structure, but they become significantly more complicated. This looks similar to the Michael & Scott lock-free list but is missing the crucial step of checking that the tail pointer is actually pointing to the end of the list. https://www.cs.rochester.edu/research/synchronization/pseudocode/queues.html – Robert Derber Jun 15 '18 at 00:45
  • I suspected it might actually be an attempt at some kind of lock-free queue implementation based on a list, but it wasn't clear to me that that is the OP's intent. Students doing their homework assignments often find code that isn't really of the type they are looking for and try to bend it. They also often mix up their terminology. The OP's title mentions a list and the code seems to incorrectly implements a FIFO. I am not willing to guess at this point. – jwdonahue Jun 16 '18 at 16:28
  • } while (!__sync_bool_compare_and_swap(&(q->next), NULL, p)); __sync_bool_compare_and_swap( &(_list->tail), q, p); Race condition here – user1079475 Aug 18 '20 at 11:20

0 Answers0