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

[Langer, Christoph]

Hi Matthias,

looks good to me.

Best regards
Christoph

> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> Baesken, Matthias
> Sent: Freitag, 21. Dezember 2018 08:01
> To: David Holmes <david.holmes at oracle.com>; 'hotspot-
> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Subject: RFR [XS] 8215707: [macosx] fix pthread_getschedparam and
> pthread_setschedparam calls - was : RE: RFR [XS] : 8215707: [macosx] check
> return code of pthread_getschedparam
> 
> 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
> >