Following up on a suggestion from Xiaobin Lu a while back, I have prepared diffs to remove Interruptible IO support from bsd-port. The reasons are as follows: 1) Interruptible IO in Solaris is depreciated and will be removed at some time in the future. 2) It is broken in bsd-port and never worked. I missed at least one code fragment needed to enable this. Since the 1.6 hotspot BSD port was based on my openjdk7 hotspot port, it is also broken in all BSD 1.6 ports. The last time Interruptible IO support worked on BSD was our 1.5 ports. 3) This is the only hotspot feature that came from the Solaris code base so removing it will make the BSD hotspot code base closer to the Linux code base. Attached are patches to the hotspot, jdk and corba workspaces that remove Interruptible IO. Please test and report any problems to the list. diff -r a0e379de9c84 make/common/Defs-bsd.gmk --- a/make/common/Defs-bsd.gmk Sun Jun 07 08:32:26 2009 -0700 +++ b/make/common/Defs-bsd.gmk Tue Jun 09 12:18:10 2009 -0400 @@ -261,7 +261,7 @@ override LIBSOCKET = override LIBTHREAD = override MOOT_PRIORITIES = true -override NO_INTERRUPTIBLE_IO = false +override NO_INTERRUPTIBLE_IO = true override OPENWIN_HOME = $(X11_PATH) override OPENWIN_LIB = $(OPENWIN_HOME)/lib override OTHER_M4FLAGS = -D__GLIBC__ -DGNU_ASSEMBLER diff -r ffe40030c236 src/os/bsd/vm/hpi_bsd.hpp --- a/src/os/bsd/vm/hpi_bsd.hpp Sun Jun 07 08:33:32 2009 -0700 +++ b/src/os/bsd/vm/hpi_bsd.hpp Tue Jun 09 12:17:37 2009 -0400 @@ -40,11 +40,15 @@ } inline size_t hpi::read(int fd, void *buf, unsigned int nBytes) { - INTERRUPTIBLE_RETURN_INT(::read(fd, buf, (size_t) nBytes), os::Bsd::clear_interrupted); + size_t res; + RESTARTABLE( (size_t) ::read(fd, buf, (size_t) nBytes), res); + return res; } inline size_t hpi::write(int fd, const void *buf, unsigned int nBytes) { - INTERRUPTIBLE_RETURN_INT(::write(fd, buf, (size_t) nBytes), os::Bsd::clear_interrupted); + size_t res; + RESTARTABLE((size_t) ::write(fd, buf, (size_t) nBytes), res); + return res; } @@ -59,11 +63,11 @@ } inline int hpi::recv(int fd, char *buf, int nBytes, int flags) { - INTERRUPTIBLE_RETURN_INT(::recv(fd, buf, nBytes, (unsigned int) flags), os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::recv(fd, buf, nBytes, (unsigned int) flags)); } inline int hpi::send(int fd, char *buf, int nBytes, int flags) { - INTERRUPTIBLE_RETURN_INT(::send(fd, buf, nBytes, (unsigned int) flags), os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::send(fd, buf, nBytes, (unsigned int) flags)); } inline int hpi::raw_send(int fd, char *buf, int nBytes, int flags) { @@ -79,12 +83,11 @@ for(;;) { struct pollfd pfd; - int res; pfd.fd = fd; pfd.events = POLLIN | POLLERR; - INTERRUPTIBLE_NORESTART(::poll(&pfd, 1, timeout), res, os::Bsd::clear_interrupted); + int res = ::poll(&pfd, 1, timeout); if (res == OS_ERR && errno == EINTR) { @@ -108,26 +111,7 @@ } inline int hpi::connect(int fd, struct sockaddr *him, int len) { - do { - int _result; - INTERRUPTIBLE_NORESTART(::connect(fd, him, len), _result, - os::Bsd::clear_interrupted); - - // Depending on when thread interruption is reset, _result could be - // one of two values when errno == EINTR - - if (((_result == OS_INTRPT) || (_result == OS_ERR)) && (errno == EINTR)) { - /* restarting a connect() changes its errno semantics */ - INTERRUPTIBLE(::connect(fd, him, len), _result, - os::Bsd::clear_interrupted); - /* undo these changes */ - if (_result == OS_ERR) { - if (errno == EALREADY) errno = EINPROGRESS; /* fall through */ - else if (errno == EISCONN) { errno = 0; return OS_OK; } - } - } - return _result; - } while(false); + RESTARTABLE_RETURN_INT(::connect(fd, him, len)); } inline int hpi::accept(int fd, struct sockaddr *him, int *len) { @@ -136,21 +120,19 @@ // fetch it's value, this cast is safe for now. The java.net group // may need and want to change this interface someday if socklen_t goes // to 64 bits on some platform that we support. - // Bsd doc says this can't return EINTR, unlike accept() on Solaris + // At least OpenBSD and FreeBSD can return EINTR from accept. - if (fd < 0) - return OS_ERR; - INTERRUPTIBLE_RETURN_INT((int)::accept(fd, him, (socklen_t*) len), os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::accept(fd, him, (socklen_t *)len)); } inline int hpi::recvfrom(int fd, char *buf, int nBytes, int flags, sockaddr *from, int *fromlen) { - INTERRUPTIBLE_RETURN_INT((int)::recvfrom(fd, buf, nBytes, (unsigned int) flags, from, (socklen_t *)fromlen), os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::recvfrom(fd, buf, nBytes, (unsigned int) flags, from, (socklen_t *)fromlen)); } inline int hpi::sendto(int fd, char *buf, int len, int flags, struct sockaddr *to, int tolen) { - INTERRUPTIBLE_RETURN_INT((int)::sendto(fd, buf, len, (unsigned int) flags, to, tolen),os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::sendto(fd, buf, len, (unsigned int) flags, to, tolen)); } inline int hpi::socket_available(int fd, jint *pbytes) { @@ -190,7 +172,7 @@ (fd, him, len)); */ inline int hpi::bind(int fd, struct sockaddr *him, int len){ - INTERRUPTIBLE_RETURN_INT_NORESTART(::bind(fd, him, len),os::Bsd::clear_interrupted); + return ::bind(fd, him, len); } /* diff -r ffe40030c236 src/os/bsd/vm/osThread_bsd.hpp --- a/src/os/bsd/vm/osThread_bsd.hpp Sun Jun 07 08:33:32 2009 -0700 +++ b/src/os/bsd/vm/osThread_bsd.hpp Tue Jun 09 12:17:37 2009 -0400 @@ -139,9 +139,6 @@ private: Monitor* _startThread_lock; // sync parent and child in thread creation -#ifdef _ALLBSD_SOURCE - JavaThreadState _saved_interrupt_thread_state; // the thread state before a system call -- restored afterward -#endif public: @@ -149,11 +146,6 @@ return _startThread_lock; } -#ifdef _ALLBSD_SOURCE - JavaThreadState saved_interrupt_thread_state() { return _saved_interrupt_thread_state; } - void set_saved_interrupt_thread_state(JavaThreadState state) { _saved_interrupt_thread_state = state; } -#endif - // *************************************************************** // Platform dependent initialization and cleanup // *************************************************************** diff -r ffe40030c236 src/os/bsd/vm/os_bsd.cpp --- a/src/os/bsd/vm/os_bsd.cpp Sun Jun 07 08:33:32 2009 -0700 +++ b/src/os/bsd/vm/os_bsd.cpp Tue Jun 09 12:17:37 2009 -0400 @@ -122,75 +122,6 @@ static int SR_signum = SIGUSR2; sigset_t SR_sigset; -#ifdef _ALLBSD_SOURCE -// XXXBSD: ported from solaris version -// interruptible infrastructure - -// setup_interruptible saves the thread state before going into an -// interruptible system call. -// The saved state is used to restore the thread to -// its former state whether or not an interrupt is received. -// Used by classloader os::read -// hpi calls skip this layer and stay in _thread_in_native - -void os::Bsd::setup_interruptible(JavaThread* thread) { - - JavaThreadState thread_state = thread->thread_state(); - - assert(thread_state != _thread_blocked, "Coming from the wrong thread"); - assert(thread_state != _thread_in_native, "Native threads skip setup_interruptible"); - OSThread* osthread = thread->osthread(); - osthread->set_saved_interrupt_thread_state(thread_state); - thread->frame_anchor()->make_walkable(thread); - ThreadStateTransition::transition(thread, thread_state, _thread_blocked); -} - -// Version of setup_interruptible() for threads that are already in -// _thread_blocked. Used by os_sleep(). -void os::Bsd::setup_interruptible_already_blocked(JavaThread* thread) { - thread->frame_anchor()->make_walkable(thread); -} - -JavaThread* os::Bsd::setup_interruptible() { - JavaThread* thread = (JavaThread*)ThreadLocalStorage::thread(); - setup_interruptible(thread); - return thread; -} - -#ifdef ASSERT - -JavaThread* os::Bsd::setup_interruptible_native() { - JavaThread* thread = (JavaThread*)ThreadLocalStorage::thread(); - JavaThreadState thread_state = thread->thread_state(); - assert(thread_state == _thread_in_native, "Assumed thread_in_native"); - return thread; -} - -void os::Bsd::cleanup_interruptible_native(JavaThread* thread) { - JavaThreadState thread_state = thread->thread_state(); - assert(thread_state == _thread_in_native, "Assumed thread_in_native"); -} -#endif - -// cleanup_interruptible reverses the effects of setup_interruptible -// setup_interruptible_already_blocked() does not need any cleanup. - -void os::Bsd::cleanup_interruptible(JavaThread* thread) { - OSThread* osthread = thread->osthread(); - - ThreadStateTransition::transition(thread, _thread_blocked, osthread->saved_interrupt_thread_state()); -} - -// I/O interruption related counters called in _INTERRUPTIBLE - -void os::Bsd::bump_interrupted_before_count() { - RuntimeService::record_interrupted_before_count(); -} - -void os::Bsd::bump_interrupted_during_count() { - RuntimeService::record_interrupted_during_count(); -} -#endif //////////////////////////////////////////////////////////////////////////////// // utility functions @@ -3106,7 +3037,7 @@ } size_t os::read(int fd, void *buf, unsigned int nBytes) { - INTERRUPTIBLE_RETURN_INT_VM(::read(fd, buf, nBytes), os::Bsd::clear_interrupted); + RESTARTABLE_RETURN_INT(::read(fd, buf, nBytes)); } // TODO-FIXME: reconcile Solaris' os::sleep with the bsd variation. diff -r ffe40030c236 src/os/bsd/vm/os_bsd.hpp --- a/src/os/bsd/vm/os_bsd.hpp Sun Jun 07 08:33:32 2009 -0700 +++ b/src/os/bsd/vm/os_bsd.hpp Tue Jun 09 12:17:37 2009 -0400 @@ -98,27 +98,6 @@ static void rebuild_cpu_to_node_map(); static GrowableArray<int>* cpu_to_node() { return _cpu_to_node; } public: -#ifdef _ALLBSD_SOURCE -//XXXBSD: ported from solaris version - enum { - clear_interrupted = true - }; - static void setup_interruptible(JavaThread* thread); - static void setup_interruptible_already_blocked(JavaThread* thread); - static JavaThread* setup_interruptible(); - static void cleanup_interruptible(JavaThread* thread); - - // perf counter incrementers used by _INTERRUPTIBLE - - static void bump_interrupted_before_count(); - static void bump_interrupted_during_count(); - -#ifdef ASSERT - static JavaThread* setup_interruptible_native(); - static void cleanup_interruptible_native(JavaThread* thread); -#endif - -#endif static void init_thread_fpu_state(); #ifndef _ALLBSD_SOURCE diff -r ffe40030c236 src/os/bsd/vm/os_bsd.inline.hpp --- a/src/os/bsd/vm/os_bsd.inline.hpp Sun Jun 07 08:33:32 2009 -0700 +++ b/src/os/bsd/vm/os_bsd.inline.hpp Tue Jun 09 12:17:37 2009 -0400 @@ -117,107 +117,6 @@ return ::closedir(dirp); } -//XXXBSD: ported from solaris version -////////////////////////////////////////////////////////////////////////////// -//////////////////////////////////////////////////////////////////////////////// - -// macros for interruptible io and system calls and system call restarting - -#define _INTERRUPTIBLE(_setup, _cmd, _result, _thread, _clear, _before, _after, _int_enable) \ -do { \ - _setup; \ - _before; \ - OSThread* _osthread = _thread->osthread(); \ - if (_int_enable && _thread->has_last_Java_frame()) { \ - /* this is java interruptible io stuff */ \ - if (os::is_interrupted(_thread, _clear)) { \ - os::Bsd::bump_interrupted_before_count(); \ - _result = OS_INTRPT; \ - } else { \ - /* _cmd always expands to an assignment to _result */ \ - if ((_cmd) < 0 && errno == EINTR \ - && os::is_interrupted(_thread, _clear)) { \ - os::Bsd::bump_interrupted_during_count(); \ - _result = OS_INTRPT; \ - } \ - } \ - } else { \ - /* this is normal blocking io stuff */ \ - _cmd; \ - } \ - _after; \ -} while(false) - -// Interruptible io support + restarting of interrupted system calls - -#ifndef ASSERT - -#define INTERRUPTIBLE(_cmd, _result, _clear) do { \ - _INTERRUPTIBLE( JavaThread* _thread = (JavaThread*)ThreadLocalStorage::thread(),_result = _cmd, _result, _thread, _clear, , , UseVMInterruptibleIO); \ -} while((_result == OS_ERR) && (errno == EINTR)) - -#else - -// This adds an assertion that it is only called from thread_in_native -// The call overhead is skipped for performance in product mode -#define INTERRUPTIBLE(_cmd, _result, _clear) do { \ - _INTERRUPTIBLE(JavaThread* _thread = os::Bsd::setup_interruptible_native(), _result = _cmd, _result, _thread, _clear, , os::Bsd::cleanup_interruptible_native(_thread), UseVMInterruptibleIO ); \ -} while((_result == OS_ERR) && (errno == EINTR)) - -#endif - -// Used for calls from _thread_in_vm, not from _thread_in_native -#define INTERRUPTIBLE_VM(_cmd, _result, _clear) do { \ - _INTERRUPTIBLE(JavaThread* _thread = os::Bsd::setup_interruptible(), _result = _cmd, _result, _thread, _clear, , os::Bsd::cleanup_interruptible(_thread), UseVMInterruptibleIO ); \ -} while((_result == OS_ERR) && (errno == EINTR)) - -/* Use NORESTART when the system call cannot return EINTR, when something other - than a system call is being invoked, or when the caller must do EINTR - handling. */ - -#ifndef ASSERT - -#define INTERRUPTIBLE_NORESTART(_cmd, _result, _clear) \ - _INTERRUPTIBLE( JavaThread* _thread = (JavaThread*)ThreadLocalStorage::thread(),_result = _cmd, _result, _thread, _clear, , , UseVMInterruptibleIO) - -#else - -// This adds an assertion that it is only called from thread_in_native -// The call overhead is skipped for performance in product mode -#define INTERRUPTIBLE_NORESTART(_cmd, _result, _clear) \ - _INTERRUPTIBLE(JavaThread* _thread = os::Bsd::setup_interruptible_native(), _result = _cmd, _result, _thread, _clear, , os::Bsd::cleanup_interruptible_native(_thread), UseVMInterruptibleIO ) - -#endif - -// Don't attend to UseVMInterruptibleIO. Always allow interruption. -// Also assumes that it is called from the _thread_blocked state. -// Used by os_sleep(). - -#define INTERRUPTIBLE_NORESTART_VM_ALWAYS(_cmd, _result, _thread, _clear) \ - _INTERRUPTIBLE(os::Bsd::setup_interruptible_already_blocked(_thread), _result = _cmd, _result, _thread, _clear, , , true ) - -#define INTERRUPTIBLE_RETURN_INT(_cmd, _clear) do { \ - int _result; \ - do { \ - INTERRUPTIBLE(_cmd, _result, _clear); \ - } while((_result == OS_ERR) && (errno == EINTR)); \ - return _result; \ -} while(false) - -#define INTERRUPTIBLE_RETURN_INT_VM(_cmd, _clear) do { \ - int _result; \ - do { \ - INTERRUPTIBLE_VM(_cmd, _result, _clear); \ - } while((_result == OS_ERR) && (errno == EINTR)); \ - return _result; \ -} while(false) - -#define INTERRUPTIBLE_RETURN_INT_NORESTART(_cmd, _clear) do { \ - int _result; \ - INTERRUPTIBLE_NORESTART(_cmd, _result, _clear); \ - return _result; \ -} while(false) - // macros for restartable system calls #define RESTARTABLE(_cmd, _result) do { \ diff -r 2b8eb56d5cf2 make/common/Defs-bsd.gmk --- a/make/common/Defs-bsd.gmk Sun Jun 07 08:34:03 2009 -0700 +++ b/make/common/Defs-bsd.gmk Tue Jun 09 12:18:02 2009 -0400 @@ -284,7 +284,7 @@ override LIBSOCKET = override LIBTHREAD = override MOOT_PRIORITIES = true -override NO_INTERRUPTIBLE_IO = false +override NO_INTERRUPTIBLE_IO = true override OPENWIN_HOME = $(X11_PATH) override OPENWIN_LIB = $(OPENWIN_HOME)/lib override OTHER_M4FLAGS = -D__GLIBC__ -DGNU_ASSEMBLER diff -r 2b8eb56d5cf2 src/solaris/hpi/native_threads/src/sys_api_td.c --- a/src/solaris/hpi/native_threads/src/sys_api_td.c Sun Jun 07 08:34:03 2009 -0700 +++ b/src/solaris/hpi/native_threads/src/sys_api_td.c Tue Jun 09 12:18:02 2009 -0400 @@ -57,10 +57,6 @@ #error If there was no policy change, this could be a makefile error. #endif -#if defined(_ALLBSD_SOURCE) && defined(NO_INTERRUPTIBLE_IO) -#error If there was no policy change, this could be a makefile error. -#endif - #ifdef NO_INTERRUPTIBLE_IO #undef CLOSEIO #else