Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Ask a question #2

Merged
merged 3 commits into from about 2 years ago

2 participants

Kezhu Wang 云风
Kezhu Wang

Is there any case that ((struct socket *)s)->status was set to SOCKET_CLOSED, but s->node is not NULL ?

I think there is no chance that socket's node field get new value when its status is already set to CLOSED.
So in commit 5d2bba5, I didn't free socket's node field in mread_pull().

云风
Owner

I don't think move "ringbuffer_free" to "_close_client" is right,

Because ringbuffer_collect has already clear the block list ( set blk->id to -1 ) .

Your are right. Thanks.

云风
Owner

It has not to reset the rb->head when EWOULDBLOCK , because the block not link to the list , so it can be used later. and we hardly got EWOULDBLOCK actually. (epoll tell the socket can be read first)

What's your idea ?

This commit does several things:
1. set newly allocated id to -1. So if client acquire it, but without using, ringbuffer can reuse it later.
2. give back newly allocated block if we don't use it. I think ringbuffer is pleasure, when client do this.
3. handle SOCKET_SUSPEND situation.

Resetting rb->head when EWOULDBLOCK conforms to point 2. Nothing about whether EWOULDBLOCK can happen.

云风
Owner

You are right about the status SOCKET_CLOSED .

云风 cloudwu merged commit b75b6d0 into from April 10, 2012
云风 cloudwu closed this April 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 2 changed files with 22 additions and 12 deletions. Show diff stats Hide diff stats

  1. 29  mread.c
  2. 5  ringbuffer.c
29  mread.c
@@ -25,6 +25,8 @@
25 25
 #define SOCKET_READ 3
26 26
 #define SOCKET_POLLIN 4
27 27
 
  28
+#define SOCKET_ALIVE	SOCKET_SUSPEND
  29
+
28 30
 struct socket {
29 31
 	int fd;
30 32
 	struct ringbuffer_block * node;
@@ -144,7 +146,7 @@ mread_close(struct mread_pool *self) {
144 146
 	int i;
145 147
 	struct socket * s = self->sockets;
146 148
 	for (i=0;i<self->max_connection;i++) {
147  
-		if (s[i].status != SOCKET_INVALID) {
  149
+		if (s[i].status >= SOCKET_ALIVE) {
148 150
 			close(s[i].fd);
149 151
 		}
150 152
 	}
@@ -291,6 +293,8 @@ static void
291 293
 _close_client(struct mread_pool * self, int id) {
292 294
 	struct socket * s = &self->sockets[id];
293 295
 	s->status = SOCKET_CLOSED;
  296
+	ringbuffer_free(self->rb, s->temp);
  297
+	ringbuffer_free(self->rb, s->node);
294 298
 	s->node = NULL;
295 299
 	s->temp = NULL;
296 300
 	close(s->fd);
@@ -303,9 +307,6 @@ _close_client(struct mread_pool * self, int id) {
303 307
 static void
304 308
 _close_active(struct mread_pool * self) {
305 309
 	int id = self->active;
306  
-	struct socket * s = &self->sockets[id];
307  
-	ringbuffer_free(self->rb, s->temp);
308  
-	ringbuffer_free(self->rb, s->node);
309 310
 	_close_client(self, id);
310 311
 }
311 312
 
@@ -334,17 +335,16 @@ mread_pull(struct mread_pool * self , int size) {
334 335
 		self->skip += size;
335 336
 		return buffer;
336 337
 	}
337  
-	if (s->status == SOCKET_CLOSED) {
338  
-		ringbuffer_free(self->rb , s->node);
339  
-		s->node = NULL;
340  
-		return NULL;
341  
-	}
342  
-
343  
-	if (s->status == SOCKET_READ) {
  338
+	switch (s->status) {
  339
+	case SOCKET_READ:
344 340
 		s->status = SOCKET_SUSPEND;
  341
+	case SOCKET_CLOSED:
  342
+	case SOCKET_SUSPEND:
345 343
 		return NULL;
  344
+	default:
  345
+		assert(s->status == SOCKET_POLLIN);
  346
+		break;
346 347
 	}
347  
-	assert(s->status == SOCKET_POLLIN);
348 348
 
349 349
 	int sz = size - rd_size;
350 350
 	int rd = READBLOCKSIZE;
@@ -380,16 +380,20 @@ mread_pull(struct mread_pool * self , int size) {
380 380
 			break;
381 381
 		}
382 382
 		if (bytes == 0) {
  383
+			ringbuffer_resize(rb, blk, 0);
383 384
 			_close_active(self);
384 385
 			return NULL;
385 386
 		}
386 387
 		if (bytes == -1) {
387 388
 			switch(errno) {
388 389
 			case EWOULDBLOCK:
  390
+				ringbuffer_resize(rb, blk, 0);
  391
+				s->status = SOCKET_SUSPEND;
389 392
 				return NULL;
390 393
 			case EINTR:
391 394
 				continue;
392 395
 			default:
  396
+				ringbuffer_resize(rb, blk, 0);
393 397
 				_close_active(self);
394 398
 				return NULL;
395 399
 			}
@@ -406,6 +410,7 @@ mread_pull(struct mread_pool * self , int size) {
406 410
 	struct ringbuffer_block * temp = ringbuffer_alloc(rb, size);
407 411
 	while (temp == NULL) {
408 412
 		int collect_id = ringbuffer_collect(rb);
  413
+		_close_client(self , collect_id);
409 414
 		if (id == collect_id) {
410 415
 			return NULL;
411 416
 		}
5  ringbuffer.c
@@ -67,6 +67,7 @@ _alloc(struct ringbuffer * rb, int total_size , int size) {
67 67
 	blk->length = sizeof(struct ringbuffer_block) + size;
68 68
 	blk->offset = 0;
69 69
 	blk->next = -1;
  70
+	blk->id = -1;
70 71
 	struct ringbuffer_block * next = block_next(rb, blk);
71 72
 	rb->head = block_offset(rb, next);
72 73
 	if (align_length < total_size) {
@@ -129,6 +130,10 @@ ringbuffer_collect(struct ringbuffer * rb) {
129 130
 
130 131
 void
131 132
 ringbuffer_resize(struct ringbuffer * rb, struct ringbuffer_block * blk, int size) {
  133
+	if (size == 0) {
  134
+		rb->head = block_offset(rb, blk);
  135
+		return;
  136
+	}
132 137
 	int align_length = ALIGN(sizeof(struct ringbuffer_block) + size);
133 138
 	int old_length = ALIGN(blk->length);
134 139
 	assert(align_length < old_length);
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.