Merge lp:~eday/gearmand/bug-fixes into lp:gearmand/1.0

Proposed by Eric Day on 2010-04-04
Status: Merged
Merged at revision: not available
Proposed branch: lp:~eday/gearmand/bug-fixes
Merge into: lp:gearmand/1.0
Diff against target: 338 lines (+168/-11) (has conflicts)
8 files modified
.bzrignore (+1/-0)
ChangeLog (+10/-0)
libgearman-server/job.c (+23/-1)
libgearman-server/job.h (+2/-1)
libgearman-server/server.c (+11/-6)
libgearman/connection.c (+2/-2)
libgearman/universal.c (+1/-1)
tests/worker_test.c (+118/-0)
Text conflict in ChangeLog
To merge this branch: bzr merge lp:~eday/gearmand/bug-fixes
Reviewer Review Type Date Requested Status
Gearman-developers 2010-04-04 Pending
Review via email: mp+22792@code.launchpad.net
To post a comment you must log in.
Brian Aker (brianaker) wrote :

Hi!

Memory leak:

http://paste2.org/p/757002

I was in the middle of packaging up a release. Do you want to fix this up for it?

Cheers,
   -Brian

lp:~eday/gearmand/bug-fixes updated on 2010-04-04
337. By Eric Day on 2010-04-04

Fixed memory leaks in abandoned_worker test.

Eric Day (eday) wrote :

This should be fixed now.

-Eric

On Sun, Apr 04, 2010 at 10:00:15PM -0000, Brian Aker wrote:
> Hi!
>
> Memory leak:
>
> http://paste2.org/p/757002
>
> I was in the middle of packaging up a release. Do you want to fix this up for it?
>
> Cheers,
> -Brian
>
> --
> https://code.launchpad.net/~eday/gearmand/bug-fixes/+merge/22792
> You are the owner of lp:~eday/gearmand/bug-fixes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2010-02-16 18:34:41 +0000
3+++ .bzrignore 2010-04-04 22:10:45 +0000
4@@ -31,6 +31,7 @@
5 config/ltmain.sh
6 config/missing
7 config/plugin.ac
8+config/pandora_vc_revinfo
9 configure
10 configure.h
11 docs/api
12
13=== modified file 'ChangeLog'
14--- ChangeLog 2010-04-04 01:18:12 +0000
15+++ ChangeLog 2010-04-04 22:10:45 +0000
16@@ -1,9 +1,19 @@
17+<<<<<<< TREE
18 0.13 Sat Apr 3 18:06:23 PDT 2010
19 * Fix for bug #518512.
20 * Use OR REPLACE syntax for inserting items into the sqlite.
21 * Changed default bitfield optimize setting to only be off for Solaris.
22 * Refactoring on server.
23
24+=======
25+0.13 - 2010-04-??
26+ * Various bug fixes as reported on mailing list, such as a bad return
27+ code and treating EHOSTDOWN as lost connection for FreeBSD.
28+ * Fixed bug #543402 so jobs taking more than max job retries will
29+ be removed from persistentn queue as well.
30+ * Check to make sure worker job results match assigned worker.
31+
32+>>>>>>> MERGE-SOURCE
33 0.12 Thu Feb 18 11:28:49 PST 2010
34 * Fixed bug where memory loss occured if data was too large.
35 * Added gearman_strerror().
36
37=== modified file 'libgearman-server/job.c'
38--- libgearman-server/job.c 2010-04-02 02:04:04 +0000
39+++ libgearman-server/job.c 2010-04-04 22:10:45 +0000
40@@ -287,7 +287,8 @@
41 }
42
43 gearman_server_job_st *gearman_server_job_get(gearman_server_st *server,
44- const char *job_handle)
45+ const char *job_handle,
46+ gearman_server_con_st *worker_con)
47 {
48 uint32_t key;
49
50@@ -299,6 +300,13 @@
51 if (server_job->job_handle_key == key &&
52 !strcmp(server_job->job_handle, job_handle))
53 {
54+ /* Check to make sure the worker asking for the job still owns the job. */
55+ if (worker_con != NULL &&
56+ (server_job->worker == NULL || server_job->worker->con != worker_con))
57+ {
58+ return NULL;
59+ }
60+
61 return server_job;
62 }
63 }
64@@ -434,6 +442,20 @@
65 return ret;
66 }
67
68+ /* Remove from persistent queue if one exists. */
69+ if (job->state & GEARMAN_SERVER_JOB_QUEUED &&
70+ job->server->queue_done_fn != NULL)
71+ {
72+ ret= (*(job->server->queue_done_fn))(job->server,
73+ (void *)job->server->queue_context,
74+ job->unique,
75+ (size_t)strlen(job->unique),
76+ job->function->function_name,
77+ job->function->function_name_size);
78+ if (ret != GEARMAN_SUCCESS)
79+ return ret;
80+ }
81+
82 gearman_server_job_free(job);
83 return GEARMAN_SUCCESS;
84 }
85
86=== modified file 'libgearman-server/job.h'
87--- libgearman-server/job.h 2010-03-12 17:53:47 +0000
88+++ libgearman-server/job.h 2010-04-04 22:10:45 +0000
89@@ -89,7 +89,8 @@
90 */
91 GEARMAN_API
92 gearman_server_job_st *gearman_server_job_get(gearman_server_st *server,
93- const char *job_handle);
94+ const char *job_handle,
95+ gearman_server_con_st *worker_con);
96
97 /**
98 * See if there are any jobs to be run for the server worker connection.
99
100=== modified file 'libgearman-server/server.c'
101--- libgearman-server/server.c 2010-04-02 02:04:04 +0000
102+++ libgearman-server/server.c 2010-04-04 22:10:45 +0000
103@@ -314,7 +314,7 @@
104 snprintf(job_handle, GEARMAN_JOB_HANDLE_SIZE, "%.*s",
105 (uint32_t)(packet->arg_size[0]), (char *)(packet->arg[0]));
106
107- server_job= gearman_server_job_get(server_con->thread->server, job_handle);
108+ server_job= gearman_server_job_get(server_con->thread->server, job_handle, NULL);
109
110 /* Queue status result packet. */
111 if (server_job == NULL)
112@@ -473,7 +473,8 @@
113 case GEARMAN_COMMAND_WORK_DATA:
114 case GEARMAN_COMMAND_WORK_WARNING:
115 server_job= gearman_server_job_get(server_con->thread->server,
116- (char *)(packet->arg[0]));
117+ (char *)(packet->arg[0]),
118+ server_con);
119 if (server_job == NULL)
120 {
121 return _server_error_packet(server_con, "job_not_found",
122@@ -489,7 +490,8 @@
123
124 case GEARMAN_COMMAND_WORK_STATUS:
125 server_job= gearman_server_job_get(server_con->thread->server,
126- (char *)(packet->arg[0]));
127+ (char *)(packet->arg[0]),
128+ server_con);
129 if (server_job == NULL)
130 {
131 return _server_error_packet(server_con, "job_not_found",
132@@ -523,7 +525,8 @@
133
134 case GEARMAN_COMMAND_WORK_COMPLETE:
135 server_job= gearman_server_job_get(server_con->thread->server,
136- (char *)(packet->arg[0]));
137+ (char *)(packet->arg[0]),
138+ server_con);
139 if (server_job == NULL)
140 {
141 return _server_error_packet(server_con, "job_not_found",
142@@ -555,7 +558,8 @@
143
144 case GEARMAN_COMMAND_WORK_EXCEPTION:
145 server_job= gearman_server_job_get(server_con->thread->server,
146- (char *)(packet->arg[0]));
147+ (char *)(packet->arg[0]),
148+ server_con);
149 if (server_job == NULL)
150 {
151 return _server_error_packet(server_con, "job_not_found",
152@@ -574,7 +578,8 @@
153 snprintf(job_handle, GEARMAN_JOB_HANDLE_SIZE, "%.*s",
154 (uint32_t)(packet->arg_size[0]), (char *)(packet->arg[0]));
155
156- server_job= gearman_server_job_get(server_con->thread->server, job_handle);
157+ server_job= gearman_server_job_get(server_con->thread->server, job_handle,
158+ server_con);
159 if (server_job == NULL)
160 {
161 return _server_error_packet(server_con, "job_not_found",
162
163=== modified file 'libgearman/connection.c'
164--- libgearman/connection.c 2010-03-09 22:17:12 +0000
165+++ libgearman/connection.c 2010-04-04 22:10:45 +0000
166@@ -616,7 +616,7 @@
167 }
168 else if (errno == EINTR)
169 continue;
170- else if (errno == EPIPE || errno == ECONNRESET)
171+ else if (errno == EPIPE || errno == ECONNRESET || errno == EHOSTDOWN)
172 {
173 if (! (connection->options.ignore_lost_connection))
174 {
175@@ -875,7 +875,7 @@
176 }
177 else if (errno == EINTR)
178 continue;
179- else if (errno == EPIPE || errno == ECONNRESET)
180+ else if (errno == EPIPE || errno == ECONNRESET || errno == EHOSTDOWN)
181 {
182 if (! (connection->options.ignore_lost_connection))
183 {
184
185=== modified file 'libgearman/universal.c'
186--- libgearman/universal.c 2010-01-07 17:46:44 +0000
187+++ libgearman/universal.c 2010-04-04 22:10:45 +0000
188@@ -371,7 +371,7 @@
189 gearman_packet_free(&packet);
190 _pop_non_blocking(universal, orig_block_universal);
191
192- return GEARMAN_SUCCESS;
193+ return ret;
194 }
195
196 void gearman_free_all_packets(gearman_universal_st *universal)
197
198=== modified file 'tests/worker_test.c'
199--- tests/worker_test.c 2010-01-08 17:32:18 +0000
200+++ tests/worker_test.c 2010-04-04 22:10:45 +0000
201@@ -18,6 +18,8 @@
202 #include <string.h>
203 #include <unistd.h>
204
205+#define GEARMAN_CORE
206+
207 #include <libgearman/gearman.h>
208
209 #include "test.h"
210@@ -355,6 +357,121 @@
211 return TEST_SUCCESS;
212 }
213
214+static test_return_t abandoned_worker_test(void *object __attribute__((unused)))
215+{
216+ gearman_client_st client;
217+ char job_handle[GEARMAN_JOB_HANDLE_SIZE];
218+ gearman_return_t ret;
219+ gearman_universal_st gearman;
220+ gearman_connection_st worker1;
221+ gearman_connection_st worker2;
222+ gearman_packet_st packet;
223+ const void *args[2];
224+ size_t args_size[2];
225+
226+ assert(gearman_client_create(&client) != NULL);
227+ gearman_client_add_server(&client, NULL, WORKER_TEST_PORT);
228+ ret= gearman_client_do_background(&client, "abandoned_worker", NULL, NULL, 0,
229+ job_handle);
230+ if (ret != GEARMAN_SUCCESS)
231+ {
232+ printf("abandoned_worker_test:%s\n", gearman_client_error(&client));
233+ return TEST_FAILURE;
234+ }
235+ gearman_client_free(&client);
236+
237+ /* Now take job with one worker. */
238+ if (gearman_universal_create(&gearman, NULL) == NULL)
239+ return TEST_FAILURE;
240+
241+ if (gearman_connection_create(&gearman, &worker1, NULL) == NULL)
242+ return TEST_FAILURE;
243+
244+ gearman_connection_set_host(&worker1, NULL, WORKER_TEST_PORT);
245+
246+ args[0]= "abandoned_worker";
247+ args_size[0]= strlen("abandoned_worker");
248+ if (gearman_packet_create_args(&gearman, &packet, GEARMAN_MAGIC_REQUEST,
249+ GEARMAN_COMMAND_CAN_DO,
250+ args, args_size, 1) != GEARMAN_SUCCESS)
251+ {
252+ return TEST_FAILURE;
253+ }
254+
255+ if (gearman_connection_send(&worker1, &packet, true) != GEARMAN_SUCCESS)
256+ return TEST_FAILURE;
257+
258+ gearman_packet_free(&packet);
259+
260+ if (gearman_packet_create_args(&gearman, &packet, GEARMAN_MAGIC_REQUEST,
261+ GEARMAN_COMMAND_GRAB_JOB,
262+ NULL, NULL, 0) != GEARMAN_SUCCESS)
263+ {
264+ return TEST_FAILURE;
265+ }
266+
267+ if (gearman_connection_send(&worker1, &packet, true) != GEARMAN_SUCCESS)
268+ return TEST_FAILURE;
269+
270+ gearman_packet_free(&packet);
271+
272+ gearman_connection_recv(&worker1, &packet, &ret, false);
273+ if (ret != GEARMAN_SUCCESS || packet.command != GEARMAN_COMMAND_JOB_ASSIGN)
274+ return TEST_FAILURE;
275+
276+ if (strcmp(job_handle, packet.arg[0]))
277+ {
278+ printf("unexpected job: %s != %s\n", job_handle, (char *)packet.arg[0]);
279+ return TEST_FAILURE;
280+ }
281+
282+ gearman_packet_free(&packet);
283+
284+ if (gearman_connection_create(&gearman, &worker2, NULL) == NULL)
285+ return TEST_FAILURE;
286+
287+ gearman_connection_set_host(&worker2, NULL, WORKER_TEST_PORT);
288+
289+ args[0]= "abandoned_worker";
290+ args_size[0]= strlen("abandoned_worker");
291+ if (gearman_packet_create_args(&gearman, &packet, GEARMAN_MAGIC_REQUEST,
292+ GEARMAN_COMMAND_CAN_DO,
293+ args, args_size, 1) != GEARMAN_SUCCESS)
294+ {
295+ return TEST_FAILURE;
296+ }
297+
298+ if (gearman_connection_send(&worker2, &packet, true) != GEARMAN_SUCCESS)
299+ return TEST_FAILURE;
300+
301+ gearman_packet_free(&packet);
302+
303+ args[0]= job_handle;
304+ args_size[0]= strlen(job_handle) + 1;
305+ args[1]= "test";
306+ args_size[1]= 4;
307+ if (gearman_packet_create_args(&gearman, &packet, GEARMAN_MAGIC_REQUEST,
308+ GEARMAN_COMMAND_WORK_COMPLETE,
309+ args, args_size, 2) != GEARMAN_SUCCESS)
310+ {
311+ return TEST_FAILURE;
312+ }
313+
314+ if (gearman_connection_send(&worker2, &packet, true) != GEARMAN_SUCCESS)
315+ return TEST_FAILURE;
316+
317+ gearman_packet_free(&packet);
318+
319+ gearman_universal_set_timeout(&gearman, 1000);
320+ gearman_connection_recv(&worker2, &packet, &ret, false);
321+ if (ret != GEARMAN_SUCCESS || packet.command != GEARMAN_COMMAND_ERROR)
322+ return TEST_FAILURE;
323+
324+ gearman_packet_free(&packet);
325+ gearman_universal_free(&gearman);
326+
327+ return TEST_SUCCESS;
328+}
329
330 static void *_gearman_worker_add_function_worker_fn(gearman_job_st *job __attribute__((unused)),
331 void *context __attribute__((unused)),
332@@ -579,6 +696,7 @@
333 {"gearman_worker_work with timout", 0, gearman_worker_work_with_test },
334 {"gearman_worker_context", 0, gearman_worker_context_test },
335 {"echo_max", 0, echo_max_test },
336+ {"abandoned_worker", 0, abandoned_worker_test },
337 {0, 0, 0}
338 };
339

Subscribers

People subscribed via source and target branches

to all changes: