On Wed, 9 Mar 2022 08:35:41 GMT, Johannes Bechberger <d...@openjdk.java.net> 
wrote:

>> The WXMode for the current thread (on MacOS aarch64) is currently stored in 
>> the thread class which is unnecessary as the WXMode is bound to the current 
>> OS thread, not the current instance of the thread class.
>> This pull request moves the storage of the current WXMode into a thread 
>> local global variable in `os` and changes all related code. SafeFetch 
>> depended on the existence of a thread object only because of the WXMode. 
>> This pull request therefore removes the dependency, making SafeFetch usable 
>> in more contexts.
>
> Johannes Bechberger has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   current_thread_wx -> ThreadWX

The change proposes to assume WXWrite as the initial state. Have you considered 
to extend ThreadWXEnable to fix the assert failure? Something like below (I 
have not tried to compile though). The refactoring looks OK, but it makes sense 
to separate it from functional change.


class ThreadWXEnable  {
  Thread* _thread;
  WXMode _old_mode;

public:
  ThreadWXEnable(WXMode new_mode, Thread* thread) :
    _thread(thread)
  {
    if (_thread) {
      _old_mode = _thread->enable_wx(new_mode);
    } else {
      os::current_thread_enable_wx(new_mode);
      _old_mode = WXWrite;
    }
  }
  ~ThreadWXEnable() {
    if (_thread) {
      _thread->enable_wx(_old_mode);
    } else {
      os::current_thread_enable_wx(_old_mode);
    }
  }
};

-------------

PR: https://git.openjdk.java.net/jdk/pull/7727

Reply via email to