Klaus Demo bjoern / 98ab7af
Fix an infinite loop caused by misuse of Unix API Plus some refactoring. Closes #88. The issue was that we checked for "read(...) <= 0", and then "errno not in {EAGAIN, EWOULDBLOCK}". Sometimes we would run into "read(...) == 0, errno in {EAGAIN, EWOULDBLOCK}". In this case we must NOT continue reading from the client, but rather close the connection. Indeed, it is illegal to check errno unless read returns -1, because errno is never actually zeroed out and may still have an "old" value from some other syscall. Our case distinction relied on errno be zeroed out. Jonas Haag 4 years ago
1 changed file(s) with 79 addition(s) and 56 deletion(s). Raw diff Collapse all Expand all
2828 "HTTP/1.1 500 Internal Server Error\r\n\r\n"
2929 };
3030
31 enum write_state {
31 enum _rw_state {
3232 not_yet_done = 1,
3333 done,
3434 aborted,
3535 };
36 typedef enum _rw_state read_state;
37 typedef enum _rw_state write_state;
3638
3739 typedef struct {
3840 ServerInfo* server_info;
4951 static ev_io_callback ev_io_on_request;
5052 static ev_io_callback ev_io_on_read;
5153 static ev_io_callback ev_io_on_write;
52 static enum write_state on_write_sendfile(struct ev_loop*, Request*);
53 static enum write_state on_write_chunk(struct ev_loop*, Request*);
54 static write_state on_write_sendfile(struct ev_loop*, Request*);
55 static write_state on_write_chunk(struct ev_loop*, Request*);
5456 static bool do_send_chunk(Request*);
5557 static bool do_sendfile(Request*);
5658 static bool handle_nonzero_errno(Request*);
59 static void close_connection(struct ev_loop*, Request*);
5760
5861
5962 void server_run(ServerInfo* server_info)
142145 static char read_buf[READ_BUFFER_SIZE];
143146
144147 Request* request = REQUEST_FROM_WATCHER(watcher);
148 read_state read_state;
145149
146150 ssize_t read_bytes = read(
147151 request->client_fd,
151155
152156 GIL_LOCK(0);
153157
154 if(read_bytes <= 0) {
155 if(errno != EAGAIN && errno != EWOULDBLOCK) {
156 if(read_bytes == 0)
157 DBG_REQ(request, "Client disconnected");
158 else
159 DBG_REQ(request, "Hit errno %d while read()ing", errno);
160 close(request->client_fd);
161 ev_io_stop(mainloop, &request->ev_watcher);
162 Request_free(request);
163 }
164 goto out;
165 }
166
167 Request_parse(request, read_buf, (size_t)read_bytes);
168
169 if(request->state.error_code) {
170 DBG_REQ(request, "Parse error");
171 request->current_chunk = PyString_FromString(
172 http_error_messages[request->state.error_code]);
173 assert(request->iterator == NULL);
174 }
175 else if(request->state.parse_finished) {
176 if(!wsgi_call_application(request)) {
177 assert(PyErr_Occurred());
178 PyErr_Print();
179 assert(!request->state.chunked_response);
180 Py_XCLEAR(request->iterator);
158 if (read_bytes == 0) {
159 /* Client disconnected */
160 read_state = aborted;
161 DBG_REQ(request, "Client disconnected");
162 } else if (read_bytes < 0) {
163 /* Would block or error */
164 if(errno == EAGAIN || errno == EWOULDBLOCK) {
165 read_state = not_yet_done;
166 } else {
167 read_state = aborted;
168 DBG_REQ(request, "Hit errno %d while read()ing", errno);
169 }
170 } else {
171 /* OK, either expect more data or done reading */
172 Request_parse(request, read_buf, (size_t)read_bytes);
173 if(request->state.error_code) {
174 /* HTTP parse error */
175 read_state = done;
176 DBG_REQ(request, "Parse error");
181177 request->current_chunk = PyString_FromString(
182 http_error_messages[HTTP_SERVER_ERROR]);
183 }
184 } else {
185 /* Wait for more data */
186 goto out;
187 }
188
189 ev_io_stop(mainloop, &request->ev_watcher);
190 ev_io_init(&request->ev_watcher, &ev_io_on_write,
191 request->client_fd, EV_WRITE);
192 ev_io_start(mainloop, &request->ev_watcher);
193
194 out:
178 http_error_messages[request->state.error_code]);
179 assert(request->iterator == NULL);
180 } else if(request->state.parse_finished) {
181 /* HTTP parse successful */
182 read_state = done;
183 bool wsgi_ok = wsgi_call_application(request);
184 if (!wsgi_ok) {
185 /* Response is "HTTP 500 Internal Server Error" */
186 DBG_REQ(request, "WSGI app error");
187 assert(PyErr_Occurred());
188 PyErr_Print();
189 assert(!request->state.chunked_response);
190 Py_XCLEAR(request->iterator);
191 request->current_chunk = PyString_FromString(
192 http_error_messages[HTTP_SERVER_ERROR]);
193 }
194 } else {
195 /* Wait for more data */
196 read_state = not_yet_done;
197 }
198 }
199
200 switch (read_state) {
201 case not_yet_done:
202 break;
203 case done:
204 DBG_REQ(request, "Stop read watcher, start write watcher");
205 ev_io_stop(mainloop, &request->ev_watcher);
206 ev_io_init(&request->ev_watcher, &ev_io_on_write,
207 request->client_fd, EV_WRITE);
208 ev_io_start(mainloop, &request->ev_watcher);
209 break;
210 case aborted:
211 close_connection(mainloop, request);
212 break;
213 }
214
195215 GIL_UNLOCK(0);
196216 }
197217
218238
219239 GIL_LOCK(0);
220240
221 enum write_state write_state;
241 write_state write_state;
222242 if(request->state.use_sendfile) {
223243 write_state = on_write_sendfile(mainloop, request);
224244 } else {
229249 case not_yet_done:
230250 break;
231251 case done:
232 /* Done with the response. */
233 ev_io_stop(mainloop, &request->ev_watcher);
234
235252 if(request->state.keep_alive) {
236253 DBG_REQ(request, "done, keep-alive");
254 ev_io_stop(mainloop, &request->ev_watcher);
237255 Request_clean(request);
238256 Request_reset(request);
239257 ev_io_init(&request->ev_watcher, &ev_io_on_read,
241259 ev_io_start(mainloop, &request->ev_watcher);
242260 } else {
243261 DBG_REQ(request, "done, close");
244 close(request->client_fd);
245 Request_free(request);
262 close_connection(mainloop, request);
246263 }
247264 break;
248265 case aborted:
249266 /* Response was aborted due to an error. We can't do anything graceful here
250267 * because at least one chunk is already sent... just close the connection. */
251 ev_io_stop(mainloop, &request->ev_watcher);
252 close(request->client_fd);
253 Request_free(request);
268 close_connection(mainloop, request);
254269 break;
255270 }
256271
257272 GIL_UNLOCK(0);
258273 }
259274
260 static enum write_state
275 static write_state
261276 on_write_sendfile(struct ev_loop* mainloop, Request* request)
262277 {
263278 /* A sendfile response is split into two phases:
290305 }
291306
292307
293 static enum write_state
308 static write_state
294309 on_write_chunk(struct ev_loop* mainloop, Request* request)
295310 {
296311 if (do_send_chunk(request))
299314
300315 if(request->iterator) {
301316 /* Reached the end of a chunk in the response iterator. Get next chunk. */
302 PyObject* next_chunk;
303 next_chunk = wsgi_iterable_get_next_chunk(request);
317 PyObject* next_chunk = wsgi_iterable_get_next_chunk(request);
304318 if(next_chunk) {
305319 /* We found another chunk to send. */
306320 if(request->state.chunked_response) {
408422 return false;
409423 }
410424 }
425
426 static void
427 close_connection(struct ev_loop *mainloop, Request* request)
428 {
429 DBG_REQ(request, "Closing socket");
430 ev_io_stop(mainloop, &request->ev_watcher);
431 close(request->client_fd);
432 Request_free(request);
433 }