From: SourceForge.net <no...@so...> - 2009-06-29 17:49:39
|
Bugs item #219354, was opened at 2000-10-26 07:11 Message generated for change (Comment added) made by das You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=219354&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 01. Notifier Group: final: 8.2.3 Status: Closed Resolution: Out of Date Priority: 8 Private: No Submitted By: Nobody/Anonymous (nobody) Assigned to: Kevin B KENNY (kennykb) Summary: threaded notifier does not work with signals Initial Comment: OriginalBugID: 5817 Bug Version: 8.2.3 SubmitDate: '2000-06-06' LastModified: '2000-10-25' Severity: CRIT Status: UnAssn Submitter: techsupp OS: Solaris OSVersion: SunOS cs1.nyc.office.juno.com 5.6 Generic_105181-16 sun4u sparc Machine: SunOS cs1.nyc.office.juno.com 5.6 Generic_105181-16 sun4u sparc FixedDate: '2000-10-25' ClosedDate: '2000-10-25' Name: Benjamin Vitale Extensions: TclX 8.2 CustomShell: none, except the patch submitted above. Comments: I thought I had submitted another patch, but I don't see it here. anyway, it was incomplete- the select masks need to be reset in the EINTR case, otherwise ALL event handlers will fire, which is wrong. ReproducibleScript: 1. run the following script. tail -f sig.log; send a HUP signal. 2. telnet localhost 12346 #!./tclsh package require Tclx signal trap HUP hup proc hup {} { log "hupping" } set s [socket -server accept 12346] proc reader {fd} { gets $fd line puts $fd "you said $line" flush $fd } proc accept {fd addr port} { log "accepting" puts $fd hello flush $fd fileevent $fd read "reader $fd" } close stdin close stdout # close stderr set logfd [open sig.log {CREAT APPEND WRONLY}] proc log {s} { global logfd puts $logfd "[clock format [clock seconds] -format "%H:%M:%S"] $s" flush $logfd } log hello proc bgerror {s} { log $s } vwait forever ObservedBehavior: the string 'hupping' does not appear in the log until you telnet DesiredBehavior: the string 'hupping' should appear in the log as soon as you send the HUP signal, even if no socket activity is occurring. Patch: =================================================================== RCS file: /proj/juno/repository/tcl/core/tcl/unix/tclUnixNotfy.c,v retrieving revision 1.1.1.2 retrieving revision 1.3 diff -u -r1.1.1.2 -r1.3 --- tclUnixNotfy.c 2000/01/26 19:14:13 1.1.1.2 +++ tclUnixNotfy.c 2000/06/02 06:25:31 1.3 @@ -897,6 +897,8 @@ */ while (1) { + int async_ready = 0; + /* * Set up the select mask to include the receive pipe. */ @@ -941,6 +943,12 @@ * Try again immediately on an error. */ + if (errno == EINTR && Tcl_AsyncReady ()) + { + async_ready = 1; + memset ((void *) &masks [0], 0, 3 * MASK_SIZE * sizeof (fd_mask)); + } + else continue; } @@ -957,7 +965,8 @@ found |= word; (((long*)(tsdPtr->readyMasks))[i]) = word; } - if (found || (tsdPtr->pollState & POLL_DONE)) { + if (found || (tsdPtr->pollState & POLL_DONE) || async_ready) { + async_ready = 0; tsdPtr->eventReady = 1; Tcl_ConditionNotify(&tsdPtr->waitCV); if (tsdPtr->onList) { PatchFiles: tclUnixNotfy.c ---------------------------------------------------------------------- >Comment By: Daniel A. Steffen (das) Date: 2009-06-29 19:49 Message: To be specific, Async.3 says When an asynchronous event occurs the code that detects the event (such as a signal handler) should call Tcl_AsyncMark with the token for the handler. I don't know if that is meant to indicate that Tcl_AsyncMark() is intended to be async-signal safe, if yes, that is currently not the case in threaded tcl... ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2009-06-29 19:43 Message: one thing that should be noted however is that the signal handler implementation in TclX appears not to be async-signal safe in the threaded-tcl case, in particular the implementation of Tcl_AsyncMark() internally calls the non async-safe pthread_mutex_lock() multiple times as well as pthread_cond_broadcast(). The Tcl docs make no guarantees that Tcl_AsyncMark() can safely be called from a signal handler (nor should they IMO). ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2009-06-29 18:54 Message: AFAICT this can be closed as out of date, Tcl_AsyncMark() handles waking up the correct thread nowadays. Specifically, TclX_SignalInit() calls Tcl_AsyncCreate(ProcessSignals, NULL) for the first interp it is loaded in and installs a signal handler that calls Tcl_AsyncMark() on that async handler. In threaded tcl, Tcl_AsyncMark() calls Tcl_ThreadAlert() which calls Tcl_AlertNotifier() which wakes up the thread that called Tcl_AsyncCreate(), so the notifier thread no longer needs to do anything to ensure the async handler is called when a signal is received. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2007-02-18 23:29 Message: Logged In: YES user_id=79902 Originator: NO AIUI, getting threaded signal handling right is required for getting a merged mainline threaded Expect, which would let us move Tcl-on-Unix to being threaded by default. Raising prio. (It's alleged that jenglish knows something about threads-and-signals issues. I know I don't.) ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2001-10-17 22:56 Message: Logged In: YES user_id=72656 This may be already handled by closed bug 219251 by davygrvy. ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2001-09-19 22:07 Message: Logged In: YES user_id=75003 Benjamin Vitale = <be...@st...>. Also note that David Gravereaux did several changes relating to Async handling for threaded tcl. This might affect the bug and/or patch in this item. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=219354&group_id=10894 |