Possible bug in ck_hp.c due to ck_hp_member_cache always returning NULL

[GaneshRapolu]

In ck_hp_reclaim there is a section

    marker = ck_hp_member_cache(global, cache, &n_hazards);
...
        if (marker != NULL &&
            ck_hp_member_scan(marker, global->degree, hazard->pointer)) {
            previous = entry;
            continue;
        }

marker is seemingly meant to be non-null if the cardinality of the non-nil hazard pointers is greater than CK_HP_CACHE.

However, in ck_hp_member_cache https://github.com/concurrencykit/ck/blob/master/src/ck_hp.c#L208

    CK_STACK_FOREACH(&global->subscribers, entry) {
        record = ck_hp_record_container(entry);
        if (ck_pr_load_int(&record->state) == CK_HP_FREE)
            continue;

        if (ck_pr_load_ptr(&record->pointers) == NULL)
            continue;

        for (i = 0; i < global->degree; i++) {
            if (hazards > CK_HP_CACHE)
                break;

            pointer = ck_pr_load_ptr(&record->pointers[i]);
            if (pointer != NULL)
                cache[hazards++] = pointer;
        }
    }

    *n_hazards = hazards;
    return (entry);

The break doesn't break out of the outer loop; only the inner one. So the return value from ck_hp_member_cache will always be NULL. This will then cause

        if (marker != NULL &&
            ck_hp_member_scan(marker, global->degree, hazard->pointer)) {
            previous = entry;
            continue;
        }

to never execute. Am I missing something or this is just buggy?

[pkhuong]

You're right, there is a bug here when there are more hazard pointers than fit in the pre-allocated array of sorted pointer. To be fair, entering this fallback path is always a performance bug :( The overflow check would also ideally be moved all the way under the null check.