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

SerfThreadFn might compile with the wrong calling convention and crash on shutdown #812

Closed
GoogleCodeExporter opened this issue Apr 6, 2015 · 3 comments

Comments

@GoogleCodeExporter
Copy link

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

As the serf url fetcher uses apr threads to create and join threads the thread 
function should have the correct signature:

http://code.google.com/p/modpagespeed/source/browse/branches/latest-stable/src/n
et/instaweb/apache/serf_url_async_fetcher.cc#789

static void* SerfThreadFn(apr_thread_t* thread_id, void* context)

Should be:
static APR_DECLARE(void*) SerfThreadFn(apr_thread_t* thread_id, void* context)

include apr.h for the APR_DECLARE

It would be better to use the ThreadSystem instance used in construction of the 
fetcher.

Original issue reported on code.google.com by kspoels...@we-amp.com on 30 Oct 2013 at 11:13

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Should this be APR_THREAD_FUNC instead?

Original comment by jmara...@google.com on 4 Nov 2013 at 9:31

  • Changed state: Accepted

Loading

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Fixed in revision 3622

Original comment by jmara...@google.com on 12 Nov 2013 at 3:16

  • Changed state: Fixed

Loading

@GoogleCodeExporter
Copy link
Author

@GoogleCodeExporter GoogleCodeExporter commented Apr 6, 2015

Original comment by jefftk@google.com on 2 Jan 2014 at 5:44

  • Added labels: Milestone-v31

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant