PATCH: Threads and SIGINT
Thu Dec 23 15:52:00 GMT 1999
>> Parts I like less. I don't like that the LINUX_THREADS define is
>> used outside the posix-threads code. I think this imlementation
>> detail should be kept entirely private to the posix threads code.
Bryce> I agree - this particular #ifdef isn't required and it can be
Bryce> Unfortunatly, I don't think its that simple. For the wait()
Bryce> case, calling cond_notify() requires that we hold the mutex
Bryce> corresponding to the monitor that the target thread is waiting
Bryce> for. We can't guarantee that we can get that mutex in a timely
Bryce> fashion, as other threads may be contending for it and holding
Bryce> it. By the time we get it there may be a completely different
Bryce> thread waiting, and we'd interrupt the wrong thread.
I don't think pthreads can check that we hold the mutex, so this isn't
an issue. However, it is an issue that multiple threads might be
waiting on the condition variable. We would have to wake up all the
threads and then arrange for the others to wait again -- this is ugly
Bryce> I agree that the interrupt-by-signal stuff is a bit of a hack,
Bryce> but it does get the job done and I think it should go in, as an
Bryce> interim solution.
Sounds ok to me.
My real concern is that the posix thread code is way too ugly
already. Maybe we should break out linux threads into its own thread
port, just to reduce the #ifdef count.
Two remaining nits:
1. The new "jthread" field in _Jv_Thread_t needs a comment.
2. Now that we have jthread, we can remove the "object" field in
struct starter in posix-threads.cc.
If you can fix these, and the LINUX_THREADS use in natThread.cc, then
I think it is ok to check in. Thanks.
>> I'd rather not use platform defines like "SOLARIS". I realize we
>> do that for Linux, but that was a mistake. Instead we could have a
>> define that would be used to disable HAVE_RECURSIVE_MUTEX, and
>> define this define in configure.in when using Solaris.
Bryce> Yes, that sounds much better.
Are you going to do this?
It should probably be a separate patch.
More information about the Java-patches