summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra G <raghavendra@gluster.com>2009-09-16 12:34:19 +0000
committerAnand V. Avati <avati@dev.gluster.com>2009-09-22 06:13:35 -0700
commit00b242e1e35b2af7ccbade982e0dae7611cc019e (patch)
treee9cda47ea9635cdff40e885ef006fd9213196e47
parent953d7146f117f19ff6c92fafaffdc79e4a14d76e (diff)
client-protocol: fix race-condition encountered while accessing fdctx
- In protocol/client, fdctx is accessed by two sets of procedures, protocol_client_mark_fd_bad falls in one set whereas the other set consists of all fops which receive fd as an argument. The way these fdctxs are got is different in these two sets. While in the former set, fdctx is accessed through conf->saved_fds, which is a list of fdctxs of fds representing opened/created files. In the latter set, fdctxs are got directly from fd through fd_ctx_get(). Now there can be race conditions between two threads executing one procedure from these two sets. As an example let us consider following scenario: A flush operation is timed out and polling thread executing protocol_client_mark_fd_bad, fuse thread executing client_release. This can happen because, immediately a reply for flush is written to fuse, a release on the same fd can be sent to glusterfs and the polling thread still might be doing cleanup. Consider following set of events: 1. fuse thread does fd_ctx_get (fd). 2. polling thread gets the same fdctx but through conf->saved_fds. 3. Now both threads go ahead and does list_del (fdctx) and eventually free fdctx. In other situations the same set events might occur and the threads executing fops other than flush in the second set might be accessing a fdctx freed in protocol_client_mark_fd_bad. Signed-off-by: Anand V. Avati <avati@dev.gluster.com> BUG: 127 (race-condition in accessing fdctx in protocol/client) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=127
-rw-r--r--xlators/protocol/client/src/client-protocol.c265
1 files changed, 218 insertions, 47 deletions
diff --git a/xlators/protocol/client/src/client-protocol.c b/xlators/protocol/client/src/client-protocol.c
index 5f34ab3e694..e5b407e24ed 100644
--- a/xlators/protocol/client/src/client-protocol.c
+++ b/xlators/protocol/client/src/client-protocol.c
@@ -90,6 +90,26 @@ ret:
client_fd_ctx_t *
+this_fd_del_ctx (fd_t *file, xlator_t *this)
+{
+ int dict_ret = -1;
+ uint64_t ctxaddr = 0;
+
+ GF_VALIDATE_OR_GOTO ("client", this, out);
+ GF_VALIDATE_OR_GOTO (this->name, file, out);
+
+ dict_ret = fd_ctx_del (file, this, &ctxaddr);
+
+ if (dict_ret < 0) {
+ ctxaddr = 0;
+ }
+
+out:
+ return (client_fd_ctx_t *)(unsigned long)ctxaddr;
+}
+
+
+client_fd_ctx_t *
this_fd_get_ctx (fd_t *file, xlator_t *this)
{
int dict_ret = -1;
@@ -1616,8 +1636,16 @@ client_readv (call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size,
int64_t remote_fd = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx, EBADFD",
@@ -1673,8 +1701,16 @@ client_writev (call_frame_t *frame, xlator_t *this, fd_t *fd,
int64_t remote_fd = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -1780,8 +1816,16 @@ client_flush (call_frame_t *frame, xlator_t *this, fd_t *fd)
int64_t remote_fd = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -1831,8 +1875,16 @@ client_fsync (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags)
int64_t remote_fd = -1;
int32_t ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -1943,6 +1995,9 @@ client_fxattrop (call_frame_t *frame, xlator_t *this, fd_t *fd,
int32_t ret = -1;
ino_t ino = 0;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
if (dict) {
dict_len = dict_serialized_length (dict);
@@ -1955,7 +2010,12 @@ client_fxattrop (call_frame_t *frame, xlator_t *this, fd_t *fd,
}
if (fd) {
- fdctx = this_fd_get_ctx (fd, this);
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. "
@@ -2098,6 +2158,9 @@ client_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd,
int ret = -1;
int64_t remote_fd = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
dict_len = dict_serialized_length (dict);
if (dict_len < 0) {
@@ -2107,7 +2170,12 @@ client_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd,
goto unwind;
}
- fdctx = this_fd_get_ctx (fd, this);
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2230,11 +2298,19 @@ client_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd,
size_t namelen = 0;
ino_t ino = 0;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
if (name)
namelen = STRLEN_0(name);
- fdctx = this_fd_get_ctx (fd, this);
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get remote fd. EBADFD",
@@ -2408,8 +2484,16 @@ client_getdents (call_frame_t *frame, xlator_t *this, fd_t *fd,
int64_t remote_fd = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2459,8 +2543,16 @@ client_readdir (call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size,
int64_t remote_fd = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2512,8 +2604,16 @@ client_fsyncdir (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags)
int64_t remote_fd = -1;
int32_t ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2616,8 +2716,16 @@ client_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd,
size_t hdrlen = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2668,8 +2776,16 @@ client_fstat (call_frame_t *frame, xlator_t *this, fd_t *fd)
size_t hdrlen = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2724,8 +2840,16 @@ client_lk (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t cmd,
int32_t gf_cmd = 0;
int32_t gf_type = 0;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
+
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
- fdctx = this_fd_get_ctx (fd, this);
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -2898,10 +3022,18 @@ client_finodelk (call_frame_t *frame, xlator_t *this, const char *volume,
int32_t gf_type = 0;
int64_t remote_fd = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
vollen = STRLEN_0(volume);
- fdctx = this_fd_get_ctx (fd, this);
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_TRACE,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -3037,13 +3169,21 @@ client_fentrylk (call_frame_t *frame, xlator_t *this, const char *volume,
size_t hdrlen = -1;
int ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
if (name)
namelen = STRLEN_0(name);
+ conf = this->private;
+
vollen = STRLEN_0(volume);
- fdctx = this_fd_get_ctx (fd, this);
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_DEBUG,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -3188,10 +3328,18 @@ client_fchmod (call_frame_t *frame, xlator_t *this, fd_t *fd, mode_t mode)
int32_t op_errno = EINVAL;
int32_t op_ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
GF_VALIDATE_OR_GOTO (this->name, fd, unwind);
- fdctx = this_fd_get_ctx (fd, this);
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
op_errno = EBADFD;
gf_log (this->name, GF_LOG_DEBUG,
@@ -3234,10 +3382,18 @@ client_fchown (call_frame_t *frame, xlator_t *this, fd_t *fd, uid_t uid,
int32_t op_errno = EINVAL;
int32_t ret = -1;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
GF_VALIDATE_OR_GOTO (this->name, fd, unwind);
- fdctx = this_fd_get_ctx (fd, this);
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
op_errno = EBADFD;
gf_log (this->name, GF_LOG_DEBUG,
@@ -3290,10 +3446,18 @@ client_setdents (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags,
struct iobref *iobref = NULL;
struct iobuf *iobuf = NULL;
client_fd_ctx_t *fdctx = NULL;
+ client_conf_t *conf = NULL;
GF_VALIDATE_OR_GOTO (this->name, fd, unwind);
- fdctx = this_fd_get_ctx (fd, this);
+ conf = this->private;
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_get_ctx (fd, this);
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL) {
gf_log (this->name, GF_LOG_DEBUG,
"(%"PRId64"): failed to get fd ctx. EBADFD",
@@ -3481,13 +3645,23 @@ client_releasedir (xlator_t *this, fd_t *fd)
GF_VALIDATE_OR_GOTO (this->name, fd, out);
conf = this->private;
- fdctx = this_fd_get_ctx (fd, this);
+
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_del_ctx (fd, this);
+ if (fdctx != NULL) {
+ list_del_init (&fdctx->sfd_pos);
+ }
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
if (fdctx == NULL){
gf_log (this->name, GF_LOG_DEBUG,
"(%"PRId64"): failed to get fd ctx.",
fd->inode->ino);
goto out;
}
+
remote_fd = fdctx->remote_fd;
hdrlen = gf_hdr_len (req, 0);
hdr = gf_hdr_new (req, 0);
@@ -3497,14 +3671,8 @@ client_releasedir (xlator_t *this, fd_t *fd)
req->fd = hton64 (remote_fd);
- {
- pthread_mutex_lock (&conf->mutex);
- {
- list_del (&fdctx->sfd_pos);
- }
- pthread_mutex_unlock (&conf->mutex);
- }
FREE (fdctx);
+
fr = create_frame (this, this->ctx->pool);
GF_VALIDATE_OR_GOTO (this->name, fr, out);
@@ -3542,13 +3710,22 @@ client_release (xlator_t *this, fd_t *fd)
conf = this->private;
- fdctx = this_fd_get_ctx (fd, this);
- if (fdctx == NULL) {
- gf_log (this->name, GF_LOG_DEBUG,
- "(%"PRId64"): failed to get fd ctx.",
- fd->inode->ino);
- goto out;
- }
+ pthread_mutex_lock (&conf->mutex);
+ {
+ fdctx = this_fd_del_ctx (fd, this);
+ if (fdctx != NULL) {
+ list_del_init (&fdctx->sfd_pos);
+ }
+ }
+ pthread_mutex_unlock (&conf->mutex);
+
+ if (fdctx == NULL) {
+ gf_log (this->name, GF_LOG_DEBUG,
+ "(%"PRId64"): failed to get fd ctx.",
+ fd->inode->ino);
+ goto out;
+ }
+
remote_fd = fdctx->remote_fd;
hdrlen = gf_hdr_len (req, 0);
hdr = gf_hdr_new (req, 0);
@@ -3557,13 +3734,6 @@ client_release (xlator_t *this, fd_t *fd)
req->fd = hton64 (remote_fd);
- {
- pthread_mutex_lock (&conf->mutex);
- {
- list_del (&fdctx->sfd_pos);
- }
- pthread_mutex_unlock (&conf->mutex);
- }
FREE (fdctx);
fr = create_frame (this, this->ctx->pool);
@@ -5665,14 +5835,15 @@ protocol_client_mark_fd_bad (xlator_t *this)
conf = this->private;
- list_for_each_entry_safe (fdctx, tmp, &conf->saved_fds, sfd_pos) {
- fd_ctx_del (fdctx->fd, this, NULL);
- list_del (&fdctx->sfd_pos);
- FREE (fdctx);
- }
-
- pthread_mutex_lock (&conf->mutex);
+ pthread_mutex_lock (&conf->mutex);
{
+ list_for_each_entry_safe (fdctx, tmp, &conf->saved_fds,
+ sfd_pos) {
+ fd_ctx_del (fdctx->fd, this, NULL);
+ list_del_init (&fdctx->sfd_pos);
+ FREE (fdctx);
+ }
+
INIT_LIST_HEAD(&conf->saved_fds);
}
pthread_mutex_unlock (&conf->mutex);