Hi noriko,

The fix looks good, just two remarks
* pagedresults_set_search_result_pb is called each time search result is freed. Should not it be called in opshared.c:722 (I find this line suspicious should not it be SLAPI_PAGED_RESULTS_INDEX ?)
* in pagedresults_set_search_result_pb, you call slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &index); . My understanding it is to reset the pr_idx (index==-1). But later you test 'index'. Did you mean slapi_pblock_'''get'''(pb, SLAPI_PAGED_RESULTS_INDEX, &index).

Thank you, Thierry!

Replying to [comment:4 tbordaz]:

Hi noriko,

The fix looks good, just two remarks
* pagedresults_set_search_result_pb is called each time search result is freed. Should not it be called in opshared.c:722 (I find this line suspicious should not it be SLAPI_PAGED_RESULTS_INDEX ?)

This is more of the protection from double free and/or NULL dereference. The search result is freed in the paged results API: pagedresults_free_one(pb->pb_conn, operation, pr_idx), so by setting NULL to SLAPI_SEARCH_RESULT_SET in pblock, no other place in the thread sharing the pblock has no chance to touch it.

• in pagedresults_set_search_result_pb, you call slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &index); . My understanding it is to reset the pr_idx (index==-1). But later you test 'index'. Did you mean slapi_pblock_'''get'''(pb, SLAPI_PAGED_RESULTS_INDEX, &index).

Agh... Thank you soooo much, Thierry. shame shame I'm running tests with the fix... A revised patch is coming.

git patch file (master) -- revised; fixed a bug pointed out by Thierry (Thank you!!) and a minor memory leak.
0001-Ticket-48299-pagedresults-when-timed-out-search-resu.patch

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1267750

Hi Noriko,

The fix looks good to me. Good catch for the leak.
ACK

Reviewed and a bug found by Thierry (Thank you!!)

Pushed to master:
3d0ae27..f90c3a6 master -> master
commit f90c3a6e1933b9cc19a51b17a038f26652c4b2bc

Pushed to 389-ds-base-1.3.4:
5bc311b..56151ed 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 56151ed75bbd63af80932fe73a512df835b17593

Pushed to 389-ds-base-1.3.3:
def55f1..10ed80c 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 10ed80cf265fbfbec0c8dda005dcb84b6f39762b

Pushed to 389-ds-base-1.2.11:
e9c84a6..54e574a 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 54e574ad5334fc9c0e27be0a57551add38d84d1e

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1630

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.