[Gluster-devel] Issue with THIS and libgfapi

[Poornima Gurusiddaiah]

Hi, 

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. 
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 
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: 
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 

Please let me know your thoughts on the same. 

Regards, 
Poornima 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20150512/ce4eee2b/attachment.html>