From: Willem R. O. <wou...@xs...> - 2011-03-06 14:27:57
Attachments:
port-leak.diff
|
As I mentioned before, there is a mach port leak in sbcl on Mac OS X. I have made a patch to fix this. However, I do not know the internals of the runtime and the design decisions, so it probably is not in a form suitable for direct inclusion. Notes: 1 - I put a function (task_self) in darwin-os.c to cache the value of mach_task_self, to avoid increasing the refcount for the task port unneededly. The function task_self should be called at least once before multiple threads can access it concurrently. I assume this happens, but this is not the most elegant solution. However I could not easily find a good place to initialize the value non lazily. 2 - I have only tested it on x86-64-darwin. I also made the same change to x86-darwin. However I could not easily find a way to build the 32 bit executable on my Mac. 3 - I can write a test case (in lisp) for the port leak. But I got a bit confused by the 'pure' vs 'impure' tests. What is the difference? Commit message: Fix mach thread port leak in darwin exception handling. Fix mach task ref count leak darwin-os.c: Added function task_self, to cache mach_task_self. darwin-os.h: declare function task_self thread.c: Use cached version of mach_task_self x86-64-darwin-os.c: In catch_exception_raise, deallocate ports that are given as arguments. In setup_mach_exception_handling_thread, use cached version of mach_task_self, also deallocates thread_p Fixed error string after mach_port_move_member. x86-darwin-os.c: Deallcoate ports in catch_exception_raise, use cached version of mach_task_self in setup_mach_exception_thread, also deallocates thread port Additional (non) related questions -------------------------------- * signal_emulation_wrapper: I noticed that at the end of the signal_emulation_wrapper an invalid assembly instruction is executed, which triggers an exception. This exception is used to restore the state of the thread. I am curious why it is done through the exception mechanism, why isn't the thread state restored right at the place the exception is triggered? This saves a trip through the kernel and makes the exception handling code simpler. * Question about the foreign function interface. (This for writing the port leak test code) I did not manage to get all the information from the mach_port_names call with the foreign function interface. My basic problem is that I can not read the output array. The C-function is declared as: kern_return_t mach_port_names ( ipc_space_t task, mach_port_name_array_t *names, // eq. int ** mach_msg_type_number_t *namesCnt, mach_port_type_array_t *types, // eq. int ** mach_msg_type_number_t *typesCnt ); I tried the following: (define-alien-routine "mach_port_names" int (task int) ; (names (* int ) :out) ; does not work, gives compile error (names (array int) :out) (namesCount int :out) ; (types (* int ) :out) ; does not work, gives compile error (types int :out) (typesCount int :out)) If I use (names (* int) : out) I get a compile error that the type (* int) can not be used as an output parameter. However if I use (names (array int) :out) the problem is that (deref names 0) gives an error (because the array size is not know.) Sorry for the confused question, but I have never used the foreign-type interface before. Regards, Wim Oudshoorn. |
From: Nikolaus D. <de...@in...> - 2011-03-06 15:37:26
|
Am 06.03.2011 um 15:27 schrieb Willem Rein Oudshoorn: > > 2 - I have only tested it on x86-64-darwin. I also > made the same change to x86-darwin. However I could > not easily find a way to build the 32 bit executable on my Mac. Just a quick note: You can build a 32 bit version by setting SBCL_ARCH to x86 before calling make.sh e.g. like so: SBCL_ARCH=x86 ./make.sh For 64 bit pass x86-64 (but this the default on OS X 10.6 anyway). Someone who knows all the possible values (e.g. for non-x86 architectures) should probably document this somewhere (README / INSTALL). Best, Niko Demmel |
From: Cyrus H. <ch...@bo...> - 2011-03-06 16:09:52
Attachments:
cyrus-mach-threads.patch
|
Hi Willem, I took a slightly different approach to part of this, which was to establish a current_mach_task variable that gets set early in the initialization and to use this instead of a function call to get the task id. Any reason why this needs to be a function instead of just a variable access? My patch only fixes the leak at thread destroy time, but, for reference, here it is. Thanks for digging into this, Cyrus |
From: Willem R. O. <wou...@xs...> - 2011-03-06 17:16:57
|
Cyrus Harmon <ch...@bo...> writes: > Hi Willem, > > I took a slightly different approach to part of this, which was to > establish a current_mach_task variable that gets set early in the > initialization and to use this instead of a function call to get the > task id. Any reason why this needs to be a function instead of just a > variable access? No reason at all, I prefer a simple variable. The reason I made it a function is because I did not know if the setup_... code was called early enough. So I erred on the 'relative' safe side by making it into a function. I am glad to see that it can be a variable. > My patch only fixes the leak at thread destroy time, but, for reference, here it is. > > Thanks for digging into this, I think it is better to also deallocate the ports in catch_exception_raise, but opinions differ. My patch does this as well. (I don't remember if the catch_exception_raise deallocates are strictly necessary, but at least it keeps the port reference counts stable over the life time of the task/thread). What is most convenient? 1 - Leave as is (your patch) 2 - I modify my patch to based upon yours (Let me know if it is checked in and where, svn? git?...) Just out of curiosity, it seems there is a bit of (almost) code duplication between x86-darwin.c and x86-64-darwin.c, is there a reason this is not put in darwin-os.c ?? Wim Oudshoorn. P.S.: I am now trying to test the 32-bit version (of my patch) but at the moment I cannot even build the unpatched 32-bit version. I build 1.0.46.15, I know it is not the newest, but I do not see any changes w.r.t. the 32 build betwen 1.0.46.15 and 1.0.46.24. I will update to the latest version and try again, but that may take a while. In the meantime, this is some information: Argh! error in cold init, halting fatal error encountered in SBCL pid 70555(tid 2684802848): %PRIMITIVE HALT called; the party is over. Welcome to LDB, a low-level debugger for the Lisp runtime environment. ldb> backtrace Backtrace: 0: Foreign function ldb_monitor, fp = 0x11ff178, ra = 0xa605 1: Foreign function lose, fp = 0x11ff1a8, ra = 0x6e60 2: Foreign function handle_trap, fp = 0x11ff1d8, ra = 0x9611 3: Foreign function signal_emulation_wrapper, fp = 0x11ff1f8, ra = 0x11b05 4: Foreign function os_get_runtime_executable_path, fp = 0x11ff228, ra = 0x116e0 5: Foreign function os_get_runtime_executable_path, fp = 0x11ff5c4, ra = 0x116e0 6: SB!IMPL::INFINITE-ERROR-PROTECTOR 7: SB!KERNEL::INTERNAL-ERROR 8: Foreign function call_into_lisp, fp = 0x11ff6a8, ra = 0x190bb 9: Foreign function funcall2, fp = 0x11ff6d8, ra = 0x3bc8 10: Foreign function interrupt_internal_error, fp = 0x11ff718, ra = 0x84ec 11: Foreign function signal_emulation_wrapper, fp = 0x11ff738, ra = 0x11b05 12: Foreign function os_get_runtime_executable_path, fp = 0x11ff768, ra = 0x116e0 13: Foreign function os_get_runtime_executable_path, fp = 0x11ffb08, ra = 0x116e0 14: COMMON-LISP::GET-INTERNAL-REAL-TIME 15: (SB!C::VARARGS-ENTRY SB!C::MAKE-SOURCE-INFO) 16: (COMMON-LISP::LAMBDA ()) 17: (COMMON-LISP::FLET SB!THREAD::WITH-RECURSIVE-LOCK-THUNK) 18: (COMMON-LISP::FLET WITHOUT-INTERRUPTS-BODY-[CALL-WITH-RECURSIVE-LOCK]324) 19: SB!THREAD::CALL-WITH-RECURSIVE-LOCK 20: (COMMON-LISP::FLET SB!C::WITH-IT) 21: SB!C::ACTUALLY-COMPILE 22: SB!C::COMPILE-IN-LEXENV 23: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::COMPILE) 24: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::SET-PPRINT-DISPATCH) 25: SB!PRETTY::!PPRINT-COLD-INIT 26: SB!KERNEL::!COLD-INIT ldb> |
From: Nikodemus S. <nik...@ra...> - 2011-03-06 17:37:31
|
> P.S.: I am now trying to test the 32-bit version (of my patch) > but at the moment I cannot even build the unpatched 32-bit version. If you've previously built a 64-bit version in the same tree, you need to run "sh clean.sh" before you do a 32-bit build, and vice versa. Cheers, -- Nikodemus |
From: Willem R. O. <wou...@xs...> - 2011-03-06 18:19:44
|
Nikodemus Siivola <nik...@ra...> writes: >> P.S.: I am now trying to test the 32-bit version (of my patch) >> but at the moment I cannot even build the unpatched 32-bit version. > > If you've previously built a 64-bit version in the same tree, you need > to run "sh clean.sh" before you do a 32-bit build, and vice versa. Thank you. I have succeeded in building the 32bit version and it works. Wim Oudshoorn. |
From: Cyrus H. <ch...@bo...> - 2011-03-06 20:24:43
|
On Mar 6, 2011, at 9:16 AM, Willem Rein Oudshoorn wrote: > Cyrus Harmon <ch...@bo...> writes: > >> Hi Willem, >> >> I took a slightly different approach to part of this, which was to >> establish a current_mach_task variable that gets set early in the >> initialization and to use this instead of a function call to get the >> task id. Any reason why this needs to be a function instead of just a >> variable access? > > No reason at all, I prefer a simple variable. The reason I made it a > function is because I did not know if the setup_... code was > called early enough. So I erred on the 'relative' safe side > by making it into a function. > I am glad to see that it can be a variable. Ok, then let's leave this is a variable for the moment, at least. >> My patch only fixes the leak at thread destroy time, but, for reference, here it is. >> >> Thanks for digging into this, > > I think it is better to also deallocate the ports in > catch_exception_raise, but opinions differ. My patch does this as well. > (I don't remember if the catch_exception_raise deallocates are strictly > necessary, but at least it keeps the port reference counts stable > over the life time of the task/thread). So my patch doesn't go far enough in that it doesn't deallocate the ports created in mach_thread_init. It seems to me that the thread destruction code is the right place for deallocating those ports, but there may be other places we want to do so as well. Skipping ahead to your question below about darwin-os.c vs x86/x86-64-darwin-os.c, I think part of the problem is that there is still support for ppc darwin, which doesn't have the mach exception handling stuff (or threads, right?). I think nyef made ppc threads work on ppc/linux, so they could probably be made to work on darwin, but I'm not about to do that... But perhaps putting the code inside the proper #ifdefs in darwin-os.c would be the way to go. On yet another axis, some of the x86-64 code (arch_os_thread_init) is actually in x86-64-bsd-os.c! All of this could use a good scrubbing. What is most convenient? > > > 1 - Leave as is (your patch) I think my patch is "correct" in so far as it goes, which is not far enough. > 2 - I modify my patch to based upon yours > (Let me know if it is checked in and where, svn? git?...) It's not checked in, but I could put it on github if that would help. I'm not opposed to your patch, in principle, other than the changes to making current_mach_task be a variable, but, in practice, I can't build with your patch :( It dies here: ; /Users/sly/projects/sbcl/sbcl.git.darwin-x86-64+sb-thread+mach-task-self/obj/from-xc/src/pcl/walk.lisp-obj-tmp written ; compilation finished in 0:00:00.510 T * (#P"obj/from-xc/src/code/show.lisp-obj" #P"obj/from-xc/src/code/early-source-location.lisp-obj" #P"obj/from-xc/src/code/early-constants.lisp-obj" #P"obj/from-xc/src/code/backq.lisp-obj" #P"obj/from-xc/src/code/globals.lisp-obj" ...) * T * [undoing binding stack and other enclosing state... done] [saving current Lisp image into output/after-xc.core: scanning space for lutexes... writing 6416 bytes from the read-only space at 0x20000000 scanning space for lutexes... writing 4064 bytes from the static space at 0x20100000 scanning space for lutexes... fatal error encountered in SBCL pid 29904(tid 140735080524960): no size function for object at 0x00800f40 (widetag 0x40) which may be due to an interaction with the :sb-after-xc-core feature. I'll try again without it and see. > Just out of curiosity, it seems there is a bit of (almost) code duplication > between x86-darwin.c and x86-64-darwin.c, is there a reason > this is not put in darwin-os.c ?? By all means, cleanup here would be welcome. hrm... I'll look into the 32-bit build here as well. Cyrus > > Wim Oudshoorn. > > P.S.: I am now trying to test the 32-bit version (of my patch) > but at the moment I cannot even build the unpatched 32-bit version. > I build 1.0.46.15, I know it is not the newest, but I do not see > any changes w.r.t. the 32 build betwen 1.0.46.15 and 1.0.46.24. > I will update to the latest version and try again, but that may > take a while. > > In the meantime, this is some information: > > > > Argh! error in cold init, halting > fatal error encountered in SBCL pid 70555(tid 2684802848): > %PRIMITIVE HALT called; the party is over. > > > Welcome to LDB, a low-level debugger for the Lisp runtime environment. > ldb> backtrace > Backtrace: > 0: Foreign function ldb_monitor, fp = 0x11ff178, ra = 0xa605 > 1: Foreign function lose, fp = 0x11ff1a8, ra = 0x6e60 > 2: Foreign function handle_trap, fp = 0x11ff1d8, ra = 0x9611 > 3: Foreign function signal_emulation_wrapper, fp = 0x11ff1f8, ra = 0x11b05 > 4: Foreign function os_get_runtime_executable_path, fp = 0x11ff228, ra = 0x116e0 > 5: Foreign function os_get_runtime_executable_path, fp = 0x11ff5c4, ra = 0x116e0 > 6: SB!IMPL::INFINITE-ERROR-PROTECTOR > 7: SB!KERNEL::INTERNAL-ERROR > 8: Foreign function call_into_lisp, fp = 0x11ff6a8, ra = 0x190bb > 9: Foreign function funcall2, fp = 0x11ff6d8, ra = 0x3bc8 > 10: Foreign function interrupt_internal_error, fp = 0x11ff718, ra = 0x84ec > 11: Foreign function signal_emulation_wrapper, fp = 0x11ff738, ra = 0x11b05 > 12: Foreign function os_get_runtime_executable_path, fp = 0x11ff768, ra = 0x116e0 > 13: Foreign function os_get_runtime_executable_path, fp = 0x11ffb08, ra = 0x116e0 > 14: COMMON-LISP::GET-INTERNAL-REAL-TIME > 15: (SB!C::VARARGS-ENTRY SB!C::MAKE-SOURCE-INFO) > 16: (COMMON-LISP::LAMBDA ()) > 17: (COMMON-LISP::FLET SB!THREAD::WITH-RECURSIVE-LOCK-THUNK) > 18: (COMMON-LISP::FLET WITHOUT-INTERRUPTS-BODY-[CALL-WITH-RECURSIVE-LOCK]324) > 19: SB!THREAD::CALL-WITH-RECURSIVE-LOCK > 20: (COMMON-LISP::FLET SB!C::WITH-IT) > 21: SB!C::ACTUALLY-COMPILE > 22: SB!C::COMPILE-IN-LEXENV > 23: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::COMPILE) > 24: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::SET-PPRINT-DISPATCH) > 25: SB!PRETTY::!PPRINT-COLD-INIT > 26: SB!KERNEL::!COLD-INIT > ldb> > > > ------------------------------------------------------------------------------ > What You Don't Know About Data Connectivity CAN Hurt You > This paper provides an overview of data connectivity, details > its effect on application quality, and explores various alternative > solutions. http://p.sf.net/sfu/progress-d2d > _______________________________________________ > Sbcl-devel mailing list > Sbc...@li... > https://lists.sourceforge.net/lists/listinfo/sbcl-devel |
From: Cyrus H. <ch...@bo...> - 2011-03-06 21:04:14
|
Hrm... building with sb-after-xc-core gets most of the way there, but fails in sb-posix: SB-POSIX-TESTS::FILENAME-DESIGNATOR.1 SB-POSIX-TESTS::OPEN.1 fatal error encounteredfatal error encountered in SBCL pid 35147 in SBCL pid 35147(tid 140735080524960)(tid 2953854976): : mach_port_allocate_name failed with return_code 268435459 mach_msg_server returned SB-POSIX-TESTS::OPEN.ERROR.1 SB-POSIX-TESTS::FCNTL.1 Test SB-POSIX-TESTS::FCNTL.FLOCK.1 failed Form: (LOCALLY (DECLARE (MUFFLE-CONDITIONS COMPILER-NOTE)) (LET ((SB-POSIX-TESTS::FLOCK (MAKE-INSTANCE 'SB-POSIX:FLOCK :TYPE SB-POSIX:F-WRLCK :WHENCE SB-POSIX:SEEK-SET :START 0 :LEN 10)) (PATHNAME "fcntl.flock.1") SB-POSIX-TESTS::KID-STATUS) (CATCH 'SB-POSIX-TESTS::TEST (WITH-OPEN-FILE (SB-POSIX-TESTS::F PATHNAME :DIRECTION :OUTPUT) (WRITE-LINE "1234567890" SB-POSIX-TESTS::F) (ASSERT (ZEROP (SB-POSIX:FCNTL SB-POSIX-TESTS::F SB-POSIX:F-SETLK SB-POSIX-TESTS::FLOCK))) (LET ((SB-POSIX-TESTS::PID (SB-POSIX:FORK))) (IF (ZEROP SB-POSIX-TESTS::PID) (PROGN (MULTIPLE-VALUE-BIND (SB-POSIX-TESTS::NOPE ERROR) (IGNORE-ERRORS (SB-POSIX:FCNTL SB-POSIX-TESTS::F SB-POSIX:F-SETLK SB-POSIX-TESTS::FLOCK)) (QUIT :UNIX-STATUS (COND ((NOT (NULL SB-POSIX-TESTS::NOPE)) 1) ((= (SB-POSIX:SYSCALL-ERRNO ERROR) SB-POSIX:EAGAIN) 42) (T 86)) :RECKLESSLY-P T))) (PROGN (SETF SB-POSIX-TESTS::KID-STATUS (SB-POSIX:WEXITSTATUS (NTH-VALUE 1 (SB-POSIX:WAITPID SB-POSIX-TESTS::PID 0)))) (THROW 'SB-POSIX-TESTS::TEST NIL)))))) SB-POSIX-TESTS::KID-STATUS)) On Mar 6, 2011, at 12:24 PM, Cyrus Harmon wrote: > > On Mar 6, 2011, at 9:16 AM, Willem Rein Oudshoorn wrote: > >> Cyrus Harmon <ch...@bo...> writes: >> >>> Hi Willem, >>> >>> I took a slightly different approach to part of this, which was to >>> establish a current_mach_task variable that gets set early in the >>> initialization and to use this instead of a function call to get the >>> task id. Any reason why this needs to be a function instead of just a >>> variable access? >> >> No reason at all, I prefer a simple variable. The reason I made it a >> function is because I did not know if the setup_... code was >> called early enough. So I erred on the 'relative' safe side >> by making it into a function. >> I am glad to see that it can be a variable. > > Ok, then let's leave this is a variable for the moment, at least. > >>> My patch only fixes the leak at thread destroy time, but, for reference, here it is. >>> >>> Thanks for digging into this, >> >> I think it is better to also deallocate the ports in >> catch_exception_raise, but opinions differ. My patch does this as well. >> (I don't remember if the catch_exception_raise deallocates are strictly >> necessary, but at least it keeps the port reference counts stable >> over the life time of the task/thread). > > So my patch doesn't go far enough in that it doesn't deallocate the ports created in mach_thread_init. It seems to me that the thread destruction code is the right place for deallocating those ports, but there may be other places we want to do so as well. > > Skipping ahead to your question below about darwin-os.c vs x86/x86-64-darwin-os.c, I think part of the problem is that there is still support for ppc darwin, which doesn't have the mach exception handling stuff (or threads, right?). I think nyef made ppc threads work on ppc/linux, so they could probably be made to work on darwin, but I'm not about to do that... But perhaps putting the code inside the proper #ifdefs in darwin-os.c would be the way to go. On yet another axis, some of the x86-64 code (arch_os_thread_init) is actually in x86-64-bsd-os.c! All of this could use a good scrubbing. > > What is most convenient? >> >> >> 1 - Leave as is (your patch) > > I think my patch is "correct" in so far as it goes, which is not far enough. > >> 2 - I modify my patch to based upon yours >> (Let me know if it is checked in and where, svn? git?...) > > It's not checked in, but I could put it on github if that would help. I'm not opposed to your patch, in principle, other than the changes to making current_mach_task be a variable, but, in practice, I can't build with your patch :( It dies here: > > ; /Users/sly/projects/sbcl/sbcl.git.darwin-x86-64+sb-thread+mach-task-self/obj/from-xc/src/pcl/walk.lisp-obj-tmp written > ; compilation finished in 0:00:00.510 > T > * > (#P"obj/from-xc/src/code/show.lisp-obj" > #P"obj/from-xc/src/code/early-source-location.lisp-obj" > #P"obj/from-xc/src/code/early-constants.lisp-obj" > #P"obj/from-xc/src/code/backq.lisp-obj" > #P"obj/from-xc/src/code/globals.lisp-obj" ...) > * > T > * [undoing binding stack and other enclosing state... done] > [saving current Lisp image into output/after-xc.core: > scanning space for lutexes... > writing 6416 bytes from the read-only space at 0x20000000 > scanning space for lutexes... > writing 4064 bytes from the static space at 0x20100000 > scanning space for lutexes... > fatal error encountered in SBCL pid 29904(tid 140735080524960): > no size function for object at 0x00800f40 (widetag 0x40) > > which may be due to an interaction with the :sb-after-xc-core feature. I'll try again without it and see. > >> Just out of curiosity, it seems there is a bit of (almost) code duplication >> between x86-darwin.c and x86-64-darwin.c, is there a reason >> this is not put in darwin-os.c ?? > > By all means, cleanup here would be welcome. > > hrm... I'll look into the 32-bit build here as well. > > Cyrus > >> >> Wim Oudshoorn. >> >> P.S.: I am now trying to test the 32-bit version (of my patch) >> but at the moment I cannot even build the unpatched 32-bit version. >> I build 1.0.46.15, I know it is not the newest, but I do not see >> any changes w.r.t. the 32 build betwen 1.0.46.15 and 1.0.46.24. >> I will update to the latest version and try again, but that may >> take a while. >> >> In the meantime, this is some information: >> >> >> >> Argh! error in cold init, halting >> fatal error encountered in SBCL pid 70555(tid 2684802848): >> %PRIMITIVE HALT called; the party is over. >> >> >> Welcome to LDB, a low-level debugger for the Lisp runtime environment. >> ldb> backtrace >> Backtrace: >> 0: Foreign function ldb_monitor, fp = 0x11ff178, ra = 0xa605 >> 1: Foreign function lose, fp = 0x11ff1a8, ra = 0x6e60 >> 2: Foreign function handle_trap, fp = 0x11ff1d8, ra = 0x9611 >> 3: Foreign function signal_emulation_wrapper, fp = 0x11ff1f8, ra = 0x11b05 >> 4: Foreign function os_get_runtime_executable_path, fp = 0x11ff228, ra = 0x116e0 >> 5: Foreign function os_get_runtime_executable_path, fp = 0x11ff5c4, ra = 0x116e0 >> 6: SB!IMPL::INFINITE-ERROR-PROTECTOR >> 7: SB!KERNEL::INTERNAL-ERROR >> 8: Foreign function call_into_lisp, fp = 0x11ff6a8, ra = 0x190bb >> 9: Foreign function funcall2, fp = 0x11ff6d8, ra = 0x3bc8 >> 10: Foreign function interrupt_internal_error, fp = 0x11ff718, ra = 0x84ec >> 11: Foreign function signal_emulation_wrapper, fp = 0x11ff738, ra = 0x11b05 >> 12: Foreign function os_get_runtime_executable_path, fp = 0x11ff768, ra = 0x116e0 >> 13: Foreign function os_get_runtime_executable_path, fp = 0x11ffb08, ra = 0x116e0 >> 14: COMMON-LISP::GET-INTERNAL-REAL-TIME >> 15: (SB!C::VARARGS-ENTRY SB!C::MAKE-SOURCE-INFO) >> 16: (COMMON-LISP::LAMBDA ()) >> 17: (COMMON-LISP::FLET SB!THREAD::WITH-RECURSIVE-LOCK-THUNK) >> 18: (COMMON-LISP::FLET WITHOUT-INTERRUPTS-BODY-[CALL-WITH-RECURSIVE-LOCK]324) >> 19: SB!THREAD::CALL-WITH-RECURSIVE-LOCK >> 20: (COMMON-LISP::FLET SB!C::WITH-IT) >> 21: SB!C::ACTUALLY-COMPILE >> 22: SB!C::COMPILE-IN-LEXENV >> 23: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::COMPILE) >> 24: (SB!C::HAIRY-ARG-PROCESSOR COMMON-LISP::SET-PPRINT-DISPATCH) >> 25: SB!PRETTY::!PPRINT-COLD-INIT >> 26: SB!KERNEL::!COLD-INIT >> ldb> >> >> >> ------------------------------------------------------------------------------ >> What You Don't Know About Data Connectivity CAN Hurt You >> This paper provides an overview of data connectivity, details >> its effect on application quality, and explores various alternative >> solutions. http://p.sf.net/sfu/progress-d2d >> _______________________________________________ >> Sbcl-devel mailing list >> Sbc...@li... >> https://lists.sourceforge.net/lists/listinfo/sbcl-devel > > > ------------------------------------------------------------------------------ > What You Don't Know About Data Connectivity CAN Hurt You > This paper provides an overview of data connectivity, details > its effect on application quality, and explores various alternative > solutions. http://p.sf.net/sfu/progress-d2d > _______________________________________________ > Sbcl-devel mailing list > Sbc...@li... > https://lists.sourceforge.net/lists/listinfo/sbcl-devel |
From: Willem R. O. <wou...@xs...> - 2011-03-07 10:57:43
Attachments:
port-leak.diff
|
Wim Oudshoorn. |
From: Cyrus H. <ch...@bo...> - 2011-03-07 18:22:03
|
This is indeed much better. I was missing the deallocate calls in catch_exception_raise in my stuff. I think we're still leaking a send right here and there, but this is indeed much better, as evidenced by MachPortDump. thanks! Cyrus On Mar 7, 2011, at 2:57 AM, Willem Rein Oudshoorn wrote: > Cyrus Harmon <ch...@bo...> writes: > >> Hrm... building with sb-after-xc-core gets most of the way there, but fails in sb-posix: >> >> SB-POSIX-TESTS::FILENAME-DESIGNATOR.1 SB-POSIX-TESTS::OPEN.1 >> fatal error encounteredfatal error encountered in SBCL pid 35147 in SBCL pid 35147(tid 140735080524960)(tid 2953854976): >> : >> mach_port_allocate_name failed with return_code 268435459 >> mach_msg_server returned > > This issue is fixed by merging your patch with mine. > The reason it failed in my patch is: > - the mach_task_self is cached > - sb-posix:fork code forks and inherits the cached value. > Your fix did not have this issue because sb-posix:fork calls > (setup-mach-exceptions) > which takes care of setting the right task port in the child process. > > I have attached an updated patch. > > diff --git a/src/runtime/darwin-os.c b/src/runtime/darwin-os.c > index 4a2bef6..236c9b0 100644 > --- a/src/runtime/darwin-os.c > +++ b/src/runtime/darwin-os.c > @@ -24,6 +24,10 @@ > #include <errno.h> > #include <dlfcn.h> > > +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > +mach_port_t current_mach_task = MACH_PORT_NULL; > +#endif > + > char * > os_get_runtime_executable_path(int external) > { > @@ -37,4 +41,3 @@ os_get_runtime_executable_path(int external) > > return copied_string(path); > } > - > diff --git a/src/runtime/darwin-os.h b/src/runtime/darwin-os.h > index 9228d1b..f40d8e1 100644 > --- a/src/runtime/darwin-os.h > +++ b/src/runtime/darwin-os.h > @@ -30,6 +30,10 @@ typedef struct ucontext os_context_t; > typedef ucontext_t os_context_t; > #endif > > +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > +extern mach_port_t current_mach_task; > +#endif > + > #define SIG_MEMORY_FAULT SIGBUS > > #define SIG_STOP_FOR_GC (SIGUSR2) > diff --git a/src/runtime/thread.c b/src/runtime/thread.c > index 535263a..88048ae 100644 > --- a/src/runtime/thread.c > +++ b/src/runtime/thread.c > @@ -29,6 +29,7 @@ > #include <mach/mach.h> > #include <mach/mach_error.h> > #include <mach/mach_types.h> > +#include "darwin-os.h" > #endif > > #include "runtime.h" > @@ -318,12 +319,12 @@ new_thread_trampoline(struct thread *th) > > #ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > FSHOW((stderr, "Deallocating mach port %x\n", THREAD_STRUCT_TO_EXCEPTION_PORT(th))); > - mach_port_move_member(mach_task_self(), > + mach_port_move_member(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th), > MACH_PORT_NULL); > - mach_port_deallocate(mach_task_self(), > + mach_port_deallocate(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th)); > - mach_port_destroy(mach_task_self(), > + mach_port_destroy(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th)); > #endif > > diff --git a/src/runtime/x86-64-darwin-os.c b/src/runtime/x86-64-darwin-os.c > index 00da75f..af14c46 100644 > --- a/src/runtime/x86-64-darwin-os.c > +++ b/src/runtime/x86-64-darwin-os.c > @@ -478,7 +478,8 @@ catch_exception_raise(mach_port_t exception_port, > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_SUCCESS; > + ret = KERN_SUCCESS; > + break; > > case EXC_BAD_INSTRUCTION: > > @@ -570,14 +571,21 @@ catch_exception_raise(mach_port_t exception_port, > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_SUCCESS; > + ret = KERN_SUCCESS; > + break; > > default: > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_INVALID_RIGHT; > + ret = KERN_INVALID_RIGHT; > } > + > + mach_port_deallocate (current_mach_task, exception_port); > + mach_port_deallocate (current_mach_task, thread); > + mach_port_deallocate (current_mach_task, task); > + > + return ret; > } > > void * > @@ -604,8 +612,10 @@ setup_mach_exception_handling_thread() > pthread_t mach_exception_handling_thread = NULL; > pthread_attr_t attr; > > + current_mach_task = mach_task_self (); > + > /* allocate a mach_port for this process */ > - ret = mach_port_allocate(mach_task_self(), > + ret = mach_port_allocate(current_mach_task, > MACH_PORT_RIGHT_PORT_SET, > &mach_exception_handler_port_set); > > @@ -630,11 +640,13 @@ kern_return_t > mach_thread_init(mach_port_t thread_exception_port) > { > kern_return_t ret; > + mach_port_t thread_self; > + > /* allocate a named port for the thread */ > > FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); > > - ret = mach_port_allocate_name(mach_task_self(), > + ret = mach_port_allocate_name(current_mach_task, > MACH_PORT_RIGHT_RECEIVE, > thread_exception_port); > if (ret) { > @@ -642,7 +654,7 @@ mach_thread_init(mach_port_t thread_exception_port) > } > > /* establish the right for the thread_exception_port to send messages */ > - ret = mach_port_insert_right(mach_task_self(), > + ret = mach_port_insert_right(current_mach_task, > thread_exception_port, > thread_exception_port, > MACH_MSG_TYPE_MAKE_SEND); > @@ -650,7 +662,8 @@ mach_thread_init(mach_port_t thread_exception_port) > lose("mach_port_insert_right failed with return_code %d\n", ret); > } > > - ret = thread_set_exception_ports(mach_thread_self(), > + thread_self = mach_thread_self (); > + ret = thread_set_exception_ports(thread_self, > EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, > thread_exception_port, > EXCEPTION_DEFAULT, > @@ -658,12 +671,13 @@ mach_thread_init(mach_port_t thread_exception_port) > if (ret) { > lose("thread_set_exception_port failed with return_code %d\n", ret); > } > + mach_port_deallocate (current_mach_task, thread_self); > > - ret = mach_port_move_member(mach_task_self(), > + ret = mach_port_move_member(current_mach_task, > thread_exception_port, > mach_exception_handler_port_set); > if (ret) { > - lose("mach_port_ failed with return_code %d\n", ret); > + lose("mach_port_move_member failed with return_code %d\n", ret); > } > > return ret; > diff --git a/src/runtime/x86-darwin-os.c b/src/runtime/x86-darwin-os.c > index ad41b0f..239f7a0 100644 > --- a/src/runtime/x86-darwin-os.c > +++ b/src/runtime/x86-darwin-os.c > @@ -502,6 +502,10 @@ catch_exception_raise(mach_port_t exception_port, > siginfo.si_addr = addr; > call_handler_on_thread(thread, &thread_state, signal, &siginfo, handler); > } > + mach_port_deallocate (current_mach_task, exception_port); > + mach_port_deallocate (current_mach_task, thread); > + mach_port_deallocate (current_mach_task, task); > + > return ret; > } > > @@ -533,8 +537,10 @@ setup_mach_exception_handling_thread() > pthread_t mach_exception_handling_thread = NULL; > pthread_attr_t attr; > > + current_mach_task = mach_task_self (); > + > /* allocate a mach_port for this process */ > - ret = mach_port_allocate(mach_task_self(), > + ret = mach_port_allocate(current_mach_task, > MACH_PORT_RIGHT_PORT_SET, > &mach_exception_handler_port_set); > > @@ -559,11 +565,13 @@ kern_return_t > mach_thread_init(mach_port_t thread_exception_port) > { > kern_return_t ret; > + mach_port_t thread_self; > + > /* allocate a named port for the thread */ > > FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); > > - ret = mach_port_allocate_name(mach_task_self(), > + ret = mach_port_allocate_name(current_mach_task, > MACH_PORT_RIGHT_RECEIVE, > thread_exception_port); > if (ret) { > @@ -571,7 +579,7 @@ mach_thread_init(mach_port_t thread_exception_port) > } > > /* establish the right for the thread_exception_port to send messages */ > - ret = mach_port_insert_right(mach_task_self(), > + ret = mach_port_insert_right(current_mach_task, > thread_exception_port, > thread_exception_port, > MACH_MSG_TYPE_MAKE_SEND); > @@ -579,7 +587,8 @@ mach_thread_init(mach_port_t thread_exception_port) > lose("mach_port_insert_right failed with return_code %d\n", ret); > } > > - ret = thread_set_exception_ports(mach_thread_self(), > + thread_self = mach_thread_self (); > + ret = thread_set_exception_ports(thread_self, > EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, > thread_exception_port, > EXCEPTION_DEFAULT, > @@ -587,8 +596,9 @@ mach_thread_init(mach_port_t thread_exception_port) > if (ret) { > lose("thread_set_exception_port failed with return_code %d\n", ret); > } > + mach_port_deallocate (current_mach_task, thread_self); > > - ret = mach_port_move_member(mach_task_self(), > + ret = mach_port_move_member(current_mach_task, > thread_exception_port, > mach_exception_handler_port_set); > if (ret) { > > Wim Oudshoorn. > > ------------------------------------------------------------------------------ > What You Don't Know About Data Connectivity CAN Hurt You > This paper provides an overview of data connectivity, details > its effect on application quality, and explores various alternative > solutions. http://p.sf.net/sfu/progress-d2d_______________________________________________ > Sbcl-devel mailing list > Sbc...@li... > https://lists.sourceforge.net/lists/listinfo/sbcl-devel |
From: Cyrus H. <ch...@bo...> - 2011-03-07 20:44:15
|
Hi Willem, I hope you don't mind, but I'm going to break this up and commit the patch (or functional equivalents to it) in bite size pieces, which should help us track down precisely where things go awry, should there be any problems with the patch. thanks again, Cyrus On Mar 7, 2011, at 2:57 AM, Willem Rein Oudshoorn wrote: > Cyrus Harmon <ch...@bo...> writes: > >> Hrm... building with sb-after-xc-core gets most of the way there, but fails in sb-posix: >> >> SB-POSIX-TESTS::FILENAME-DESIGNATOR.1 SB-POSIX-TESTS::OPEN.1 >> fatal error encounteredfatal error encountered in SBCL pid 35147 in SBCL pid 35147(tid 140735080524960)(tid 2953854976): >> : >> mach_port_allocate_name failed with return_code 268435459 >> mach_msg_server returned > > This issue is fixed by merging your patch with mine. > The reason it failed in my patch is: > - the mach_task_self is cached > - sb-posix:fork code forks and inherits the cached value. > Your fix did not have this issue because sb-posix:fork calls > (setup-mach-exceptions) > which takes care of setting the right task port in the child process. > > I have attached an updated patch. > > diff --git a/src/runtime/darwin-os.c b/src/runtime/darwin-os.c > index 4a2bef6..236c9b0 100644 > --- a/src/runtime/darwin-os.c > +++ b/src/runtime/darwin-os.c > @@ -24,6 +24,10 @@ > #include <errno.h> > #include <dlfcn.h> > > +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > +mach_port_t current_mach_task = MACH_PORT_NULL; > +#endif > + > char * > os_get_runtime_executable_path(int external) > { > @@ -37,4 +41,3 @@ os_get_runtime_executable_path(int external) > > return copied_string(path); > } > - > diff --git a/src/runtime/darwin-os.h b/src/runtime/darwin-os.h > index 9228d1b..f40d8e1 100644 > --- a/src/runtime/darwin-os.h > +++ b/src/runtime/darwin-os.h > @@ -30,6 +30,10 @@ typedef struct ucontext os_context_t; > typedef ucontext_t os_context_t; > #endif > > +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > +extern mach_port_t current_mach_task; > +#endif > + > #define SIG_MEMORY_FAULT SIGBUS > > #define SIG_STOP_FOR_GC (SIGUSR2) > diff --git a/src/runtime/thread.c b/src/runtime/thread.c > index 535263a..88048ae 100644 > --- a/src/runtime/thread.c > +++ b/src/runtime/thread.c > @@ -29,6 +29,7 @@ > #include <mach/mach.h> > #include <mach/mach_error.h> > #include <mach/mach_types.h> > +#include "darwin-os.h" > #endif > > #include "runtime.h" > @@ -318,12 +319,12 @@ new_thread_trampoline(struct thread *th) > > #ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER > FSHOW((stderr, "Deallocating mach port %x\n", THREAD_STRUCT_TO_EXCEPTION_PORT(th))); > - mach_port_move_member(mach_task_self(), > + mach_port_move_member(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th), > MACH_PORT_NULL); > - mach_port_deallocate(mach_task_self(), > + mach_port_deallocate(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th)); > - mach_port_destroy(mach_task_self(), > + mach_port_destroy(current_mach_task, > THREAD_STRUCT_TO_EXCEPTION_PORT(th)); > #endif > > diff --git a/src/runtime/x86-64-darwin-os.c b/src/runtime/x86-64-darwin-os.c > index 00da75f..af14c46 100644 > --- a/src/runtime/x86-64-darwin-os.c > +++ b/src/runtime/x86-64-darwin-os.c > @@ -478,7 +478,8 @@ catch_exception_raise(mach_port_t exception_port, > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_SUCCESS; > + ret = KERN_SUCCESS; > + break; > > case EXC_BAD_INSTRUCTION: > > @@ -570,14 +571,21 @@ catch_exception_raise(mach_port_t exception_port, > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_SUCCESS; > + ret = KERN_SUCCESS; > + break; > > default: > #ifdef LISP_FEATURE_SB_THREAD > thread_mutex_unlock(&mach_exception_lock); > #endif > - return KERN_INVALID_RIGHT; > + ret = KERN_INVALID_RIGHT; > } > + > + mach_port_deallocate (current_mach_task, exception_port); > + mach_port_deallocate (current_mach_task, thread); > + mach_port_deallocate (current_mach_task, task); > + > + return ret; > } > > void * > @@ -604,8 +612,10 @@ setup_mach_exception_handling_thread() > pthread_t mach_exception_handling_thread = NULL; > pthread_attr_t attr; > > + current_mach_task = mach_task_self (); > + > /* allocate a mach_port for this process */ > - ret = mach_port_allocate(mach_task_self(), > + ret = mach_port_allocate(current_mach_task, > MACH_PORT_RIGHT_PORT_SET, > &mach_exception_handler_port_set); > > @@ -630,11 +640,13 @@ kern_return_t > mach_thread_init(mach_port_t thread_exception_port) > { > kern_return_t ret; > + mach_port_t thread_self; > + > /* allocate a named port for the thread */ > > FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); > > - ret = mach_port_allocate_name(mach_task_self(), > + ret = mach_port_allocate_name(current_mach_task, > MACH_PORT_RIGHT_RECEIVE, > thread_exception_port); > if (ret) { > @@ -642,7 +654,7 @@ mach_thread_init(mach_port_t thread_exception_port) > } > > /* establish the right for the thread_exception_port to send messages */ > - ret = mach_port_insert_right(mach_task_self(), > + ret = mach_port_insert_right(current_mach_task, > thread_exception_port, > thread_exception_port, > MACH_MSG_TYPE_MAKE_SEND); > @@ -650,7 +662,8 @@ mach_thread_init(mach_port_t thread_exception_port) > lose("mach_port_insert_right failed with return_code %d\n", ret); > } > > - ret = thread_set_exception_ports(mach_thread_self(), > + thread_self = mach_thread_self (); > + ret = thread_set_exception_ports(thread_self, > EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, > thread_exception_port, > EXCEPTION_DEFAULT, > @@ -658,12 +671,13 @@ mach_thread_init(mach_port_t thread_exception_port) > if (ret) { > lose("thread_set_exception_port failed with return_code %d\n", ret); > } > + mach_port_deallocate (current_mach_task, thread_self); > > - ret = mach_port_move_member(mach_task_self(), > + ret = mach_port_move_member(current_mach_task, > thread_exception_port, > mach_exception_handler_port_set); > if (ret) { > - lose("mach_port_ failed with return_code %d\n", ret); > + lose("mach_port_move_member failed with return_code %d\n", ret); > } > > return ret; > diff --git a/src/runtime/x86-darwin-os.c b/src/runtime/x86-darwin-os.c > index ad41b0f..239f7a0 100644 > --- a/src/runtime/x86-darwin-os.c > +++ b/src/runtime/x86-darwin-os.c > @@ -502,6 +502,10 @@ catch_exception_raise(mach_port_t exception_port, > siginfo.si_addr = addr; > call_handler_on_thread(thread, &thread_state, signal, &siginfo, handler); > } > + mach_port_deallocate (current_mach_task, exception_port); > + mach_port_deallocate (current_mach_task, thread); > + mach_port_deallocate (current_mach_task, task); > + > return ret; > } > > @@ -533,8 +537,10 @@ setup_mach_exception_handling_thread() > pthread_t mach_exception_handling_thread = NULL; > pthread_attr_t attr; > > + current_mach_task = mach_task_self (); > + > /* allocate a mach_port for this process */ > - ret = mach_port_allocate(mach_task_self(), > + ret = mach_port_allocate(current_mach_task, > MACH_PORT_RIGHT_PORT_SET, > &mach_exception_handler_port_set); > > @@ -559,11 +565,13 @@ kern_return_t > mach_thread_init(mach_port_t thread_exception_port) > { > kern_return_t ret; > + mach_port_t thread_self; > + > /* allocate a named port for the thread */ > > FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); > > - ret = mach_port_allocate_name(mach_task_self(), > + ret = mach_port_allocate_name(current_mach_task, > MACH_PORT_RIGHT_RECEIVE, > thread_exception_port); > if (ret) { > @@ -571,7 +579,7 @@ mach_thread_init(mach_port_t thread_exception_port) > } > > /* establish the right for the thread_exception_port to send messages */ > - ret = mach_port_insert_right(mach_task_self(), > + ret = mach_port_insert_right(current_mach_task, > thread_exception_port, > thread_exception_port, > MACH_MSG_TYPE_MAKE_SEND); > @@ -579,7 +587,8 @@ mach_thread_init(mach_port_t thread_exception_port) > lose("mach_port_insert_right failed with return_code %d\n", ret); > } > > - ret = thread_set_exception_ports(mach_thread_self(), > + thread_self = mach_thread_self (); > + ret = thread_set_exception_ports(thread_self, > EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, > thread_exception_port, > EXCEPTION_DEFAULT, > @@ -587,8 +596,9 @@ mach_thread_init(mach_port_t thread_exception_port) > if (ret) { > lose("thread_set_exception_port failed with return_code %d\n", ret); > } > + mach_port_deallocate (current_mach_task, thread_self); > > - ret = mach_port_move_member(mach_task_self(), > + ret = mach_port_move_member(current_mach_task, > thread_exception_port, > mach_exception_handler_port_set); > if (ret) { > > Wim Oudshoorn. > > ------------------------------------------------------------------------------ > What You Don't Know About Data Connectivity CAN Hurt You > This paper provides an overview of data connectivity, details > its effect on application quality, and explores various alternative > solutions. http://p.sf.net/sfu/progress-d2d_______________________________________________ > Sbcl-devel mailing list > Sbc...@li... > https://lists.sourceforge.net/lists/listinfo/sbcl-devel |
From: Cyrus H. <ch...@bo...> - 2011-03-08 14:50:33
|
I forgot to add a section in the NEWS file, and, for some reason, commit email don't seem to be working for me, but commits 1.0.46.25-28 and .30 address the port leaks. Details are in the logs, but, briefly, the commits do the following: .25: consolidates common code from x86-64-darwin-os.c and x86-darwin-os.c into darwin-os.c .26: squirrels away the value of current_mach_task and uses that instead of calling mach_task_self repeatedly .27: deallocates the port after the mach_thread_self call and deallocates ports in catch_exception_raise (on x86-64) .28: deallocates ports in catch_exception_raise on x86 too .30: check return values from mach_deallocate_port and remove call that was trying to deallocate the exception_port -- this call was failing and I think it's bogus/not needed. Now we no longer leak send rights all over the place. The MachPortDump utility reports much nicer results during the threads tests. There are still some send rights/named ports that don't seem to go away, however: Mach ports for '41654' (41654): Name Send Receive SendOnce PortSet DeadName DNR ---- ---- ------- -------- ------- -------- --- 0x107 1 0x207 1 0x307 2 0x40f 1 0x507 1 0x60b 1 0x70b 2 0x807 2 0x903 1 0xa03 1 0xb03 1 0xc03 1 0xd03 1 0xe03 1 0xf03 1 0x1003 1 0x1103 1 0x1303 1 0x1503 1 0x1603 1 0x1703 1 0x1803 1 ... There are about 270 of these at the end of the threads tests, but this is a great improvement over the previous situation where every thread was leaking ports. Thanks for your patches! if somebody wants to add the NEWS entry, please credit Willem, otherwise I'll get to it on my next commit. Cyrus On Mar 7, 2011, at 12:44 PM, Cyrus Harmon wrote: > Hi Willem, > > I hope you don't mind, but I'm going to break this up and commit the patch (or functional equivalents to it) in bite size pieces, which should help us track down precisely where things go awry, should there be any problems with the patch. > > thanks again, > > Cyrus > > On Mar 7, 2011, at 2:57 AM, Willem Rein Oudshoorn wrote: > >> Cyrus Harmon <ch...@bo...> writes: >> >>> Hrm... building with sb-after-xc-core gets most of the way there, but fails in sb-posix: >>> >>> SB-POSIX-TESTS::FILENAME-DESIGNATOR.1 SB-POSIX-TESTS::OPEN.1 >>> fatal error encounteredfatal error encountered in SBCL pid 35147 in SBCL pid 35147(tid 140735080524960)(tid 2953854976): >>> : >>> mach_port_allocate_name failed with return_code 268435459 >>> mach_msg_server returned >> >> This issue is fixed by merging your patch with mine. >> The reason it failed in my patch is: >> - the mach_task_self is cached >> - sb-posix:fork code forks and inherits the cached value. >> Your fix did not have this issue because sb-posix:fork calls >> (setup-mach-exceptions) >> which takes care of setting the right task port in the child process. >> >> I have attached an updated patch. >> >> diff --git a/src/runtime/darwin-os.c b/src/runtime/darwin-os.c >> index 4a2bef6..236c9b0 100644 >> --- a/src/runtime/darwin-os.c >> +++ b/src/runtime/darwin-os.c >> @@ -24,6 +24,10 @@ >> #include <errno.h> >> #include <dlfcn.h> >> >> +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER >> +mach_port_t current_mach_task = MACH_PORT_NULL; >> +#endif >> + >> char * >> os_get_runtime_executable_path(int external) >> { >> @@ -37,4 +41,3 @@ os_get_runtime_executable_path(int external) >> >> return copied_string(path); >> } >> - >> diff --git a/src/runtime/darwin-os.h b/src/runtime/darwin-os.h >> index 9228d1b..f40d8e1 100644 >> --- a/src/runtime/darwin-os.h >> +++ b/src/runtime/darwin-os.h >> @@ -30,6 +30,10 @@ typedef struct ucontext os_context_t; >> typedef ucontext_t os_context_t; >> #endif >> >> +#ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER >> +extern mach_port_t current_mach_task; >> +#endif >> + >> #define SIG_MEMORY_FAULT SIGBUS >> >> #define SIG_STOP_FOR_GC (SIGUSR2) >> diff --git a/src/runtime/thread.c b/src/runtime/thread.c >> index 535263a..88048ae 100644 >> --- a/src/runtime/thread.c >> +++ b/src/runtime/thread.c >> @@ -29,6 +29,7 @@ >> #include <mach/mach.h> >> #include <mach/mach_error.h> >> #include <mach/mach_types.h> >> +#include "darwin-os.h" >> #endif >> >> #include "runtime.h" >> @@ -318,12 +319,12 @@ new_thread_trampoline(struct thread *th) >> >> #ifdef LISP_FEATURE_MACH_EXCEPTION_HANDLER >> FSHOW((stderr, "Deallocating mach port %x\n", THREAD_STRUCT_TO_EXCEPTION_PORT(th))); >> - mach_port_move_member(mach_task_self(), >> + mach_port_move_member(current_mach_task, >> THREAD_STRUCT_TO_EXCEPTION_PORT(th), >> MACH_PORT_NULL); >> - mach_port_deallocate(mach_task_self(), >> + mach_port_deallocate(current_mach_task, >> THREAD_STRUCT_TO_EXCEPTION_PORT(th)); >> - mach_port_destroy(mach_task_self(), >> + mach_port_destroy(current_mach_task, >> THREAD_STRUCT_TO_EXCEPTION_PORT(th)); >> #endif >> >> diff --git a/src/runtime/x86-64-darwin-os.c b/src/runtime/x86-64-darwin-os.c >> index 00da75f..af14c46 100644 >> --- a/src/runtime/x86-64-darwin-os.c >> +++ b/src/runtime/x86-64-darwin-os.c >> @@ -478,7 +478,8 @@ catch_exception_raise(mach_port_t exception_port, >> #ifdef LISP_FEATURE_SB_THREAD >> thread_mutex_unlock(&mach_exception_lock); >> #endif >> - return KERN_SUCCESS; >> + ret = KERN_SUCCESS; >> + break; >> >> case EXC_BAD_INSTRUCTION: >> >> @@ -570,14 +571,21 @@ catch_exception_raise(mach_port_t exception_port, >> #ifdef LISP_FEATURE_SB_THREAD >> thread_mutex_unlock(&mach_exception_lock); >> #endif >> - return KERN_SUCCESS; >> + ret = KERN_SUCCESS; >> + break; >> >> default: >> #ifdef LISP_FEATURE_SB_THREAD >> thread_mutex_unlock(&mach_exception_lock); >> #endif >> - return KERN_INVALID_RIGHT; >> + ret = KERN_INVALID_RIGHT; >> } >> + >> + mach_port_deallocate (current_mach_task, exception_port); >> + mach_port_deallocate (current_mach_task, thread); >> + mach_port_deallocate (current_mach_task, task); >> + >> + return ret; >> } >> >> void * >> @@ -604,8 +612,10 @@ setup_mach_exception_handling_thread() >> pthread_t mach_exception_handling_thread = NULL; >> pthread_attr_t attr; >> >> + current_mach_task = mach_task_self (); >> + >> /* allocate a mach_port for this process */ >> - ret = mach_port_allocate(mach_task_self(), >> + ret = mach_port_allocate(current_mach_task, >> MACH_PORT_RIGHT_PORT_SET, >> &mach_exception_handler_port_set); >> >> @@ -630,11 +640,13 @@ kern_return_t >> mach_thread_init(mach_port_t thread_exception_port) >> { >> kern_return_t ret; >> + mach_port_t thread_self; >> + >> /* allocate a named port for the thread */ >> >> FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); >> >> - ret = mach_port_allocate_name(mach_task_self(), >> + ret = mach_port_allocate_name(current_mach_task, >> MACH_PORT_RIGHT_RECEIVE, >> thread_exception_port); >> if (ret) { >> @@ -642,7 +654,7 @@ mach_thread_init(mach_port_t thread_exception_port) >> } >> >> /* establish the right for the thread_exception_port to send messages */ >> - ret = mach_port_insert_right(mach_task_self(), >> + ret = mach_port_insert_right(current_mach_task, >> thread_exception_port, >> thread_exception_port, >> MACH_MSG_TYPE_MAKE_SEND); >> @@ -650,7 +662,8 @@ mach_thread_init(mach_port_t thread_exception_port) >> lose("mach_port_insert_right failed with return_code %d\n", ret); >> } >> >> - ret = thread_set_exception_ports(mach_thread_self(), >> + thread_self = mach_thread_self (); >> + ret = thread_set_exception_ports(thread_self, >> EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, >> thread_exception_port, >> EXCEPTION_DEFAULT, >> @@ -658,12 +671,13 @@ mach_thread_init(mach_port_t thread_exception_port) >> if (ret) { >> lose("thread_set_exception_port failed with return_code %d\n", ret); >> } >> + mach_port_deallocate (current_mach_task, thread_self); >> >> - ret = mach_port_move_member(mach_task_self(), >> + ret = mach_port_move_member(current_mach_task, >> thread_exception_port, >> mach_exception_handler_port_set); >> if (ret) { >> - lose("mach_port_ failed with return_code %d\n", ret); >> + lose("mach_port_move_member failed with return_code %d\n", ret); >> } >> >> return ret; >> diff --git a/src/runtime/x86-darwin-os.c b/src/runtime/x86-darwin-os.c >> index ad41b0f..239f7a0 100644 >> --- a/src/runtime/x86-darwin-os.c >> +++ b/src/runtime/x86-darwin-os.c >> @@ -502,6 +502,10 @@ catch_exception_raise(mach_port_t exception_port, >> siginfo.si_addr = addr; >> call_handler_on_thread(thread, &thread_state, signal, &siginfo, handler); >> } >> + mach_port_deallocate (current_mach_task, exception_port); >> + mach_port_deallocate (current_mach_task, thread); >> + mach_port_deallocate (current_mach_task, task); >> + >> return ret; >> } >> >> @@ -533,8 +537,10 @@ setup_mach_exception_handling_thread() >> pthread_t mach_exception_handling_thread = NULL; >> pthread_attr_t attr; >> >> + current_mach_task = mach_task_self (); >> + >> /* allocate a mach_port for this process */ >> - ret = mach_port_allocate(mach_task_self(), >> + ret = mach_port_allocate(current_mach_task, >> MACH_PORT_RIGHT_PORT_SET, >> &mach_exception_handler_port_set); >> >> @@ -559,11 +565,13 @@ kern_return_t >> mach_thread_init(mach_port_t thread_exception_port) >> { >> kern_return_t ret; >> + mach_port_t thread_self; >> + >> /* allocate a named port for the thread */ >> >> FSHOW((stderr, "Allocating mach port %x\n", thread_exception_port)); >> >> - ret = mach_port_allocate_name(mach_task_self(), >> + ret = mach_port_allocate_name(current_mach_task, >> MACH_PORT_RIGHT_RECEIVE, >> thread_exception_port); >> if (ret) { >> @@ -571,7 +579,7 @@ mach_thread_init(mach_port_t thread_exception_port) >> } >> >> /* establish the right for the thread_exception_port to send messages */ >> - ret = mach_port_insert_right(mach_task_self(), >> + ret = mach_port_insert_right(current_mach_task, >> thread_exception_port, >> thread_exception_port, >> MACH_MSG_TYPE_MAKE_SEND); >> @@ -579,7 +587,8 @@ mach_thread_init(mach_port_t thread_exception_port) >> lose("mach_port_insert_right failed with return_code %d\n", ret); >> } >> >> - ret = thread_set_exception_ports(mach_thread_self(), >> + thread_self = mach_thread_self (); >> + ret = thread_set_exception_ports(thread_self, >> EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION, >> thread_exception_port, >> EXCEPTION_DEFAULT, >> @@ -587,8 +596,9 @@ mach_thread_init(mach_port_t thread_exception_port) >> if (ret) { >> lose("thread_set_exception_port failed with return_code %d\n", ret); >> } >> + mach_port_deallocate (current_mach_task, thread_self); >> >> - ret = mach_port_move_member(mach_task_self(), >> + ret = mach_port_move_member(current_mach_task, >> thread_exception_port, >> mach_exception_handler_port_set); >> if (ret) { >> >> Wim Oudshoorn. >> >> ------------------------------------------------------------------------------ >> What You Don't Know About Data Connectivity CAN Hurt You >> This paper provides an overview of data connectivity, details >> its effect on application quality, and explores various alternative >> solutions. http://p.sf.net/sfu/progress-d2d_______________________________________________ >> Sbcl-devel mailing list >> Sbc...@li... >> https://lists.sourceforge.net/lists/listinfo/sbcl-devel > > > ------------------------------------------------------------------------------ > What You Don't Know About Data Connectivity CAN Hurt You > This paper provides an overview of data connectivity, details > its effect on application quality, and explores various alternative > solutions. http://p.sf.net/sfu/progress-d2d > _______________________________________________ > Sbcl-devel mailing list > Sbc...@li... > https://lists.sourceforge.net/lists/listinfo/sbcl-devel |
From: Nikodemus S. <nik...@ra...> - 2011-03-10 11:59:41
|
On 8 March 2011 16:50, Cyrus Harmon <ch...@bo...> wrote: > I forgot to add a section in the NEWS file, and, for some reason, commit > email don't seem to be working for me, but commits 1.0.46.25-28 and .30 Commit emails are still broken after the Sourceforge break-in. I've been generating and sending mine out manually. Cheers, -- Nikodemus |
From: Willem R. O. <wou...@xs...> - 2011-03-08 20:29:34
|
Cyrus Harmon <ch...@bo...> writes: > .30: check return values from mach_deallocate_port and remove call > that was trying to deallocate the exception_port -- this call was > failing and I think it's bogus/not needed. Yes, I think this could be right. I am too lazy to disassemble the mach library right now. > Now we no longer leak send rights all over the place. The MachPortDump > utility reports much nicer results during the threads tests. There are > still some send rights/named ports that don't seem to go away, > however: > > Mach ports for '41654' (41654): > > Name Send Receive SendOnce PortSet DeadName DNR > ---- ---- ------- -------- ------- -------- --- > 0x107 1 > 0x207 1 > 0x307 2 > 0x40f 1 > 0x507 1 > 0x60b 1 > 0x70b 2 > 0x807 2 > 0x903 1 > 0xa03 1 > 0xb03 1 > 0xc03 1 > 0xd03 1 > 0xe03 1 > 0xf03 1 > 0x1003 1 > 0x1103 1 > 0x1303 1 > 0x1503 1 > 0x1603 1 > 0x1703 1 > 0x1803 1 > ... > Yes, I noticed this as well. In my simple test case: (loop :repeat 100000 :do (sb-thread:make-thread (lambda ()))) It ends up with about 49 ports in total, were I expect 33. [After the first thread, 33 ports are in use, after the first 10 threads 33 are still in use.] However it happens randomly, if create 100 threads it might or might not happen, but it is more or less guaranteed to happen after 100000 threads. Now I am writing this, it occurs to me that the increase is 16 (and reproducable 16.) This 16 reminds me of the pthread code new_sem_from_pool. This function will increase the pool by 16 entries each time all semaphores are in use. So my hypothesis is that these 16 additional port rights in my scenario are all semaphores (not used but kept ready in the semaphore pool.) The increase might be triggered when suddenly a few more threads are active at the same time and trigger an overflow of the semaphore pool. Here is the function copied from pthread.c (notice the PTHREAD_MACH_CALL). __private_extern__ semaphore_t new_sem_from_pool(void) { kern_return_t res; semaphore_t sem; int i; LOCK(sem_pool_lock); if (sem_pool_current == sem_pool_count) { sem_pool_count += 16; sem_pool = realloc(sem_pool, sem_pool_count * sizeof(semaphore_t)); for (i = sem_pool_current; i < sem_pool_count; i++) { PTHREAD_MACH_CALL(semaphore_create(mach_task_self(), &sem_pool[i], SYNC_POLICY_FIFO, 0), res); } } sem = sem_pool[sem_pool_current++]; UNLOCK(sem_pool_lock); return sem; } Initial testing shows that the patched commited in git works for me. Thanks to Cyrus, for improving my patch and integrating it all in sbcl. Wim Oudshoorn. |
From: Paul K. <pv...@pv...> - 2011-03-09 01:43:08
|
On 2011-03-08, at 3:29 PM, Willem Rein Oudshoorn wrote: > Thanks to Cyrus, for improving my patch and integrating it all in > sbcl. Thanks to you and Cyrus for working on a port that definitely needs some TLC. Paul Khuong |