Hi,
We recently uncovered an issue with THIS and libgfapi, it can be generalized to any process having multiple glusterfs_ctxs.
We recently uncovered an issue with THIS and libgfapi, it can be generalized to any process having multiple glusterfs_ctxs.
Before the master xlator (fuse/libgfapi) is created, all the code that access THIS will be using global_xlator object,
defined globally for the whole of the process.
defined globally for the whole of the process.
The problem is when multiple threads start modifying THIS, and overwrite thr global_xlators' ctx eg: glfs_new:
glfs_new () {
...
ctx = glusterfs_ctx_new();
glusterfs_globals_inti();
THIS = NULL; /* implies THIS = &global_xlator */
THIS->ctx = ctx;
...
}
The issue is more severe than it appears, as the other threads like epoll, timer, sigwaiter, when not executing in
fop context will always refer to the global_xlator and global_xlator->ctx. Because of the probable race condition
glfs_new () {
...
ctx = glusterfs_ctx_new();
glusterfs_globals_inti();
THIS = NULL; /* implies THIS = &global_xlator */
THIS->ctx = ctx;
...
}
The issue is more severe than it appears, as the other threads like epoll, timer, sigwaiter, when not executing in
fop context will always refer to the global_xlator and global_xlator->ctx. Because of the probable race condition
explained above we may be referring to the stale ctxs and could lead to crashes.
Probable solution:
Currently THIS is thread specific, but the global xlator object it modifies is global to all threads!!
The obvious association would be to have global_xlator per ctx instead of per process.
The changes would be as follows:
- Have a new global_xlator object in glusterfs_ctx.
- After every creation of new ctx assign
<store THIS>
THIS = new_ctx->global_xlator
<restore THIS>
- But how to set the THIS in every thread(epoll, timer etc) that gets created as a part of that ctx.
Replace all the pthread_create for the ctx threads, with gf_pthread_create:
Probable solution:
Currently THIS is thread specific, but the global xlator object it modifies is global to all threads!!
The obvious association would be to have global_xlator per ctx instead of per process.
The changes would be as follows:
- Have a new global_xlator object in glusterfs_ctx.
- After every creation of new ctx assign
<store THIS>
THIS = new_ctx->global_xlator
<restore THIS>
- But how to set the THIS in every thread(epoll, timer etc) that gets created as a part of that ctx.
Replace all the pthread_create for the ctx threads, with gf_pthread_create:
gf_pthread_create (fn,..., ctx) {
...
thr_id = pthread_create (global_thread_init, fn, ctx...);
...
}
global_thread_init (fn, ctx, args) {
THIS = ctx->global_xlator;
fn(args);
}
The other solution would be to not associate threads with ctx, instead shared among ctxs
...
thr_id = pthread_create (global_thread_init, fn, ctx...);
...
}
global_thread_init (fn, ctx, args) {
THIS = ctx->global_xlator;
fn(args);
}
The other solution would be to not associate threads with ctx, instead shared among ctxs
Please let me know your thoughts on the same.
Regards,
Poornima
_______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
