Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

library/test_search segfault on master #1671

Closed
jprobichaud opened this issue Nov 4, 2018 · 7 comments · Fixed by #1818
Closed

library/test_search segfault on master #1671

jprobichaud opened this issue Nov 4, 2018 · 7 comments · Fixed by #1818
Assignees
Labels
Bug Bug in learning semantics, critical by default In PR There is a PR waiting to be merged for this issue L2S Learning to search subsystem Lang: C++

Comments

@jprobichaud
Copy link

jprobichaud commented Nov 4, 2018

Perhaps caused by the same issue as issue "Hook interface" #1413, building vw and then doing

cd library
make
./test_search

Also results in a core dump

This is on ArchLinux, 64bit, with GCC 8.2.1 and Boost 1.68

@jprobichaud
Copy link
Author

adding print statements, I identified that the segfault happens "between" these two lines, just after the get_task_data:

     HookTask::task_data* d = sch.get_task_data<HookTask::task_data>();
     d->run_f = _search_run_fn;

It seems to be the case also when executing from python

@jprobichaud
Copy link
Author

With the recent updates to master, we now get some extra details:

$ library/test_search 


##### train() #####

warning: task should not set options except in initialize function!
terminate called after throwing an instance of 'VW::vw_exception'
  what():  using LDF and actions costs is not yet implemented; turn off action costs
Aborted (core dumped)

@jackgerrits jackgerrits added the Bug Bug in learning semantics, critical by default label Nov 30, 2018
@jprobichaud
Copy link
Author

I coulnd't have a chance to spend some time on this again, will anyone from the "official" team be able to have a look?

@JohnLangford
Copy link
Member

JohnLangford commented Jan 28, 2019 via email

@lokitoth
Copy link
Member

I've been looking at the other hook task issue and it seems like this may be the same issue.

The higher-level issue is that we explicitly disable "hook" - which seems to have been around for a while. That is fixed by removing the check in search.cc's setup(). The next-level issue is that this uses the old API for multi-line example, which fails the multi-line test. Switching to multi_ex as input fixes that. The underlying issue is that setting the right function-pointers in SearchTask's init (in libsearch.h) does not seem to actually set them in the task-object that hook-task is looking into.

Still trying to debug why that is going on.

@lokitoth lokitoth added L2S Learning to search subsystem Lang: C++ labels Feb 26, 2019
@lokitoth
Copy link
Member

lokitoth commented Feb 26, 2019

I believe I have a fix for this - just need to do a bit more validation under Linux. After that will check whether there is more work (I expect there will be, to propagate the API changes through the Python library).

Here's the branch in my fork: https://github.com/lokitoth/vowpal_wabbit/tree/dev/Fix-SearchHookTaskCppLibrary
cc @jprobichaud

@jprobichaud
Copy link
Author

jprobichaud commented Feb 26, 2019 via email

@lokitoth lokitoth self-assigned this Feb 28, 2019
@lokitoth lokitoth reopened this Feb 28, 2019
@lokitoth lokitoth added the In PR There is a PR waiting to be merged for this issue label Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug in learning semantics, critical by default In PR There is a PR waiting to be merged for this issue L2S Learning to search subsystem Lang: C++
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants