Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

SIGSEV thrown in multithread environment #155

Open
danfran opened this issue Jan 26, 2018 · 5 comments
Open

SIGSEV thrown in multithread environment #155

danfran opened this issue Jan 26, 2018 · 5 comments

Comments

@danfran
Copy link

danfran commented Jan 26, 2018

I am running SFCGAL in a multithread env with the only sfcgal function sfcgal_geometry_intersects:

void *match_counter(void *threadarg) {
    struct thread_data *tdata = (struct thread_data *) threadarg;
    printf("starting thread-%d (%zu, %zu)\n", tdata->thread_id, tdata->idx_start, tdata->idx_end);

    int counter = 0;
    for (size_t p = tdata->idx_start; p < tdata->idx_end; p++) {
        if (sfcgal_geometry_intersects(tdata->polygon, tdata->listings->array[p].location) == 1) {
            counter++;
        }
    }
    printf("thread-%d final count %d\n", tdata->thread_id, counter);
    pthread_exit(NULL);
}

and I get Process finished with exit code 139 (interrupted by signal 11: SIGSEGV).

I have excluded issues from my side, so it looks like the SFCGAL call creates the issue. Now the relative code goes straight down to CGAL.

CGAL requires some configuration to be used on multi-threading env:
https://github.com/CGAL/cgal/wiki/Concurrency-in-CGAL

Is SFCGAL meant to work in multithread env?

@mhugo
Copy link
Contributor

mhugo commented Jan 26, 2018

Hi, CGAL is meant to be thread-safe. See the "Thread Safety" section here.

The link you provided is about using the parallel version of some algorithms, which is another story.

Are you sure your arguments to sfcgal_geometry_intersects are valid and reachable during the call ?

@danfran
Copy link
Author

danfran commented Jan 26, 2018

Hi, I run the threads changing the code in this way:

void *match_counter(void *threadarg) {
    struct thread_data *tdata = (struct thread_data *) threadarg;
    printf("starting thread-%d (%zu, %zu)\n", tdata->thread_id, tdata->idx_start, tdata->idx_end);

    for (size_t p = tdata->idx_start; p < tdata->idx_end; p++) {
        void *pt = tdata->listings->array[p].location;
        printf("%lf %lf\n", sfcgal_point_x(pt), sfcgal_point_y(pt));
        printf("%zu\n", sfcgal_polygon_num_interior_rings(polygon_t));
    }
    printf("thread-%d final count %d\n", tdata->thread_id, counter);
    pthread_exit(NULL);
}

basically I replaced sfcgal_geometry_intersects with other 2 sfcgal's functions and it works ok (data are reached in somehow).
Also, if I run the code with sfcgal_geometry_intersects without any threads works fine too, but if I start a multithread env, even with only one thread, it fails.

@danfran
Copy link
Author

danfran commented Jan 27, 2018

Hi, if I change the point to intersect itself in multi thread env it works ok:

sfcgal_geometry_intersects(tdata->listings->array[p].location, tdata->listings->array[p].location)

Could it be wrong the way I build the polygon? This is how I make it:

// WGS84 coords
double polygon [][2] =
{
        {-0.172134,51.487646},
        {-0.173336,51.484546},
        {-0.168701,51.484760},
        {-0.167843,51.488394},
        {-0.172134,51.487646}
};


size_t number_vertices_polygon = sizeof(polygon)/sizeof(polygon[0]);

sfcgal_geometry_t* linestring = sfcgal_linestring_create ();

for (size_t i = 0; i < number_vertices_polygon; i++) {
	sfcgal_linestring_add_point (
		linestring,
		sfcgal_point_create_from_xy(polygon[i][0], polygon[i][1])
	);
}

sfcgal_geometry_t *polygon_t = sfcgal_polygon_create_from_exterior_ring(*linestring);

also as alternative method for *polygon_t I tried this too:

sfcgal_geometry_t * polygon_t = sfcgal_polygon_create();
sfcgal_polygon_add_interior_ring(polygon_t, linestring);

Thank you

@mhugo
Copy link
Contributor

mhugo commented Jan 29, 2018

Could you provide a minimal source code so that we can reproduce the problem ?

@lrineau
Copy link

lrineau commented Jan 29, 2018

@mhugo, in #155 (comment):

Hi, CGAL is meant to be thread-safe. See the "Thread Safety" section here.

The link you provided is about using the parallel version of some algorithms, which is another story.

He is right. CGAL is mostly thread-safe and the Concurrency_tag is not about thread-safety but about configuring a few CGAL algorithms to use multiple threads or not.

But: note that SFCGAL::Kernel is CGAL::Exact_predicates_exact_constructions_kernel that uses a graph of constructions, and reference counting. It is rather difficult not to share geometry objects between threads, and as soon as a code does that, there case be race conditions and crashes. If, for example, you read data to geometric objects from a file, in the main thread, and then copy data to other threads, then your threads share and use the same geometry objects, and that is not safe.

SFCGAL/src/Kernel.h

Lines 26 to 31 in 5499795

namespace SFCGAL {
/**
* default Kernel
*/
typedef CGAL::Exact_predicates_exact_constructions_kernel Kernel ;

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants