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.