diff --git a/RELEASES.rst b/RELEASES.rst index 11c1a15690..1450eafd04 100644 --- a/RELEASES.rst +++ b/RELEASES.rst @@ -1145,13 +1145,16 @@ Planned intended mainly for minimal compatibility with existing code using "const" (GH-360) -* Add a debugger Throw notify for errors about to be thrown, and an option - to automatically pause before an uncaught error is thrown (GH-286, GH-347) - * Add a human readable summary of object/key for rejected property operations to make error messages more useful for operations like "null.foo = 123;" (GH-210, GH-405) +* Add a debugger Throw notify for errors about to be thrown, and an option + to automatically pause before an uncaught error is thrown (GH-286, GH-347) + +* Allow debugger detached callback to call duk_debugger_attach(), previously + this clobbered some internal state (GH-399) + * Fix "debugger" statement line number off-by-one so that the debugger now correctly pauses on the debugger statement rather than after it (GH-347) diff --git a/examples/cmdline/duk_cmdline.c b/examples/cmdline/duk_cmdline.c index 9cc6875316..96532ee887 100644 --- a/examples/cmdline/duk_cmdline.c +++ b/examples/cmdline/duk_cmdline.c @@ -593,8 +593,26 @@ static int handle_interactive(duk_context *ctx) { #ifdef DUK_CMDLINE_DEBUGGER_SUPPORT static void debugger_detached(void *udata) { + duk_context *ctx = (duk_context *) udata; + (void) ctx; fprintf(stderr, "Debugger detached, udata: %p\n", (void *) udata); fflush(stderr); + +#if 0 /* For manual auto-reattach test */ + duk_trans_socket_finish(); + duk_trans_socket_init(); + duk_trans_socket_waitconn(); + fprintf(stderr, "Debugger connected, call duk_debugger_attach() and then execute requested file(s)/eval\n"); + fflush(stderr); + duk_debugger_attach(ctx, + duk_trans_socket_read_cb, + duk_trans_socket_write_cb, + duk_trans_socket_peek_cb, + duk_trans_socket_read_flush_cb, + duk_trans_socket_write_flush_cb, + debugger_detached, + (void *) ctx); +#endif } #endif @@ -706,7 +724,7 @@ static duk_context *create_duktape_heap(int alloc_provider, int debugger, int aj duk_trans_socket_read_flush_cb, duk_trans_socket_write_flush_cb, debugger_detached, - (void *) 0xbeef1234); + (void *) ctx); #else fprintf(stderr, "Warning: option --debugger ignored, no debugger support\n"); fflush(stderr); diff --git a/examples/debug-trans-socket/duk_trans_socket.c b/examples/debug-trans-socket/duk_trans_socket.c index c25fa3ea8a..a7b20b095f 100644 --- a/examples/debug-trans-socket/duk_trans_socket.c +++ b/examples/debug-trans-socket/duk_trans_socket.c @@ -29,7 +29,7 @@ static int server_sock = -1; static int client_sock = -1; /* - * Transport init + * Transport init and finish */ void duk_trans_socket_init(void) { @@ -71,6 +71,17 @@ void duk_trans_socket_init(void) { } } +void duk_trans_socket_finish(void) { + if (client_sock >= 0) { + (void) close(client_sock); + client_sock = -1; + } + if (server_sock >= 0) { + (void) close(server_sock); + server_sock = -1; + } +} + void duk_trans_socket_waitconn(void) { struct sockaddr_in addr; socklen_t sz; diff --git a/examples/debug-trans-socket/duk_trans_socket.h b/examples/debug-trans-socket/duk_trans_socket.h index 60712a2e7d..43f4a34de4 100644 --- a/examples/debug-trans-socket/duk_trans_socket.h +++ b/examples/debug-trans-socket/duk_trans_socket.h @@ -4,6 +4,7 @@ #include "duktape.h" void duk_trans_socket_init(void); +void duk_trans_socket_finish(void); void duk_trans_socket_waitconn(void); duk_size_t duk_trans_socket_read_cb(void *udata, char *buffer, duk_size_t length); duk_size_t duk_trans_socket_write_cb(void *udata, const char *buffer, duk_size_t length); diff --git a/src/duk_api_debug.c b/src/duk_api_debug.c index 27a32e64ed..17a5f0c3e7 100644 --- a/src/duk_api_debug.c +++ b/src/duk_api_debug.c @@ -53,6 +53,10 @@ DUK_EXTERNAL void duk_debugger_attach(duk_context *ctx, const char *str; duk_size_t len; + /* XXX: should there be an error or an automatic detach if + * already attached? + */ + DUK_ASSERT_CTX_VALID(ctx); DUK_ASSERT(read_cb != NULL); DUK_ASSERT(write_cb != NULL); @@ -104,7 +108,7 @@ DUK_EXTERNAL void duk_debugger_detach(duk_context *ctx) { DUK_ASSERT(thr != NULL); DUK_ASSERT(thr->heap != NULL); - /* Can be called muliple times with no harm. */ + /* Can be called multiple times with no harm. */ duk_debug_do_detach(thr->heap); } diff --git a/src/duk_debugger.c b/src/duk_debugger.c index 5785e97ad9..d31a57c848 100644 --- a/src/duk_debugger.c +++ b/src/duk_debugger.c @@ -25,22 +25,27 @@ typedef union { #define DUK__SET_CONN_BROKEN(thr) do { \ /* For now shared handler is fine. */ \ - duk_debug_do_detach((thr)->heap); \ + duk__debug_do_detach1((thr)->heap); \ } while (0) -DUK_INTERNAL void duk_debug_do_detach(duk_heap *heap) { - /* Can be called muliple times with no harm. */ +DUK_LOCAL void duk__debug_do_detach1(duk_heap *heap) { + /* Can be called multiple times with no harm. Mark the transport + * bad (dbg_read_cb == NULL) and clear state except for the detached + * callback and the udata field. The detached callback is delayed + * to the message loop so that it can be called between messages; + * this avoids corner cases related to immediate debugger reattach + * inside the detached callback. + */ + + DUK_D(DUK_DPRINT("debugger transport detaching, marking transport broken")); heap->dbg_read_cb = NULL; heap->dbg_write_cb = NULL; heap->dbg_peek_cb = NULL; heap->dbg_read_flush_cb = NULL; heap->dbg_write_flush_cb = NULL; - if (heap->dbg_detached_cb) { - heap->dbg_detached_cb(heap->dbg_udata); - } - heap->dbg_detached_cb = NULL; - heap->dbg_udata = NULL; + /* heap->dbg_detached_cb: keep */ + /* heap->dbg_udata: keep */ heap->dbg_processing = 0; heap->dbg_paused = 0; heap->dbg_state_dirty = 0; @@ -61,6 +66,32 @@ DUK_INTERNAL void duk_debug_do_detach(duk_heap *heap) { heap->dbg_breakpoints_active[0] = (duk_breakpoint *) NULL; } +DUK_LOCAL void duk__debug_do_detach2(duk_heap *heap) { + duk_debug_detached_function detached_cb; + void *detached_udata; + + /* Safe to call multiple times. */ + + detached_cb = heap->dbg_detached_cb; + detached_udata = heap->dbg_udata; + heap->dbg_detached_cb = NULL; + heap->dbg_udata = NULL; + + if (detached_cb) { + /* Careful here: state must be wiped before the call + * so that we can cleanly handle a re-attach from + * inside the callback. + */ + DUK_D(DUK_DPRINT("detached during message loop, delayed call to detached_cb")); + detached_cb(detached_udata); + } +} + +DUK_INTERNAL void duk_debug_do_detach(duk_heap *heap) { + duk__debug_do_detach1(heap); + duk__debug_do_detach2(heap); +} + /* * Debug connection peek and flush primitives */ @@ -1714,6 +1745,10 @@ DUK_LOCAL void duk__debug_process_message(duk_hthread *thr) { break; } case DUK_DBG_CMD_DETACH: { + /* The actual detached_cb call is postponed to message loop so + * we don't need any special precautions here (just skip to EOM + * on the already closed connection). + */ duk__debug_handle_detach(thr, heap); break; } @@ -1870,6 +1905,17 @@ DUK_INTERNAL duk_bool_t duk_debug_process_messages(duk_hthread *thr, duk_bool_t } duk__debug_process_message(thr); + + if (thr->heap->dbg_read_cb == NULL) { + /* Became detached during message handling (perhaps because + * of an error or by an explicit Detach). Call detached + * callback here, between messages, to avoid confusing the + * broken connection and a possible replacement (which may + * be provided by an instant reattach inside the detached + * callback). + */ + duk__debug_do_detach2(thr->heap); + } if (thr->heap->dbg_state_dirty) { /* Executed something that may have affected status, * resend. diff --git a/website/api/duk_debugger_attach.yaml b/website/api/duk_debugger_attach.yaml index 741e198682..bcd6f3f0fe 100644 --- a/website/api/duk_debugger_attach.yaml +++ b/website/api/duk_debugger_attach.yaml @@ -21,6 +21,14 @@ summary: | read_flush_cb and write_flush_cb callbacks are optional. Unimplemented callbacks are indicated using a NULL.

+
+ Since Duktape 1.4.0 the debugger detached callback is allowed to call + duk_debugger_attach() + to immediately reattach the debugger. In Duktape 1.3.0 and before the + immediate reattach potentially caused + some issues. +
+ example: | duk_debugger_attach(ctx, my_read_cb,