RFR [XS] 8215707: [macosx] fix pthread_getschedparam and pthread_setschedparam calls - was : RE: RFR [XS] : 8215707: [macosx] check return code of pthread_getschedparam

[Baesken, Matthias]

Hello David, I adjusted  the  bug synopsis and description,  the second webrev   uses  the pthread id  of the passed  Thread*  instead of  pthread_self()  :


https://bugs.openjdk.java.net/browse/JDK-8215707

http://cr.openjdk.java.net/~mbaesken/webrevs/8215707.1/



Best regards, Matthias


> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Donnerstag, 20. Dezember 2018 13:19
> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR [XS] : 8215707: [macosx] check return code of
> pthread_getschedparam
> 
> On 20/12/2018 9:56 pm, Baesken, Matthias wrote:
> >> Note this code is likely never exercised due to the default
> >> ThreadPriorityPolicy of 0.
> >
> > Hi  David,  ThreadPriorityPolicy=1 is a valid setting / product flag.
> 
> Yes it is. That doesn't mean it is actually used to any great extent.
> 
> > You find quite a few people setting  ThreadPriorityPolicy when you search
> for it,  so  the coding should better be fixed I guess .
> 
> Quite a few? I could only find a couple (Cassandra, Gatling) and they
> were on Linux. I was surprised to find how many people knew about the
> bug that allowed the root check to be by-passed.
> 
> The fact this code is incorrect on OS X indicates to me it is not
> actually being used - or by chance priority is only ever changed in the
> current thread.
> 
> >>
> >>   That seems more likely given that the code is incorrect:
> >>
> >> pthread_getschedparam(pthread_self(), &policy, &sp);
> >>
> >> this always gets the priority of the current thread, but the method need
> >> not be called on the current thread!
> >
> > Yes , true !
> > Same for  os::set_native_priority  in  os_bsd.cpp , there pthread_self()  is
> also  used instead of the  real Thread .
> >
> > 2338OSReturn os::set_native_priority(Thread* thread, int newpri) {
> > 2339  if (!UseThreadPriorities || ThreadPriorityPolicy == 0) return OS_OK;
> > 2340
> >    ....
> > 2346#elif defined(__APPLE__) || defined(__NetBSD__)
> > 2347  struct sched_param sp;
> > 2348  int policy;
> > 2349  pthread_t self = pthread_self();
> > 2350
> > 2351  if (pthread_getschedparam(self, &policy, &sp) != 0) {
> > 2352    return OS_ERR;
> > 2353  }
> 
> If you intend fixing this probably best to modify the bug synopsis and
> description.
> 
> Cheers,
> David
>