Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 31 Oct 2024 19:07:42 GMT, Chris Plummer wrote: >>> I do wonder if mutex support can be implemented for Windows with >>> Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >>> would be nice to have. Shame threads.h is not available with some Visual >>> Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >>> nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > >> > I do wonder if mutex support can be implemented for Windows with >> > Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >> > would be nice to have. Shame threads.h is not available with some Visual >> > Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >> > nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > > Feel free to if you'd like to. Personally I don't consider it to be that > important. Bumping, @plummercj I found that casting to void in MUTEX_LOCK and MUTEX_UNLOCK also works, and it seems neater to me. Is this ok with you? - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2467690492
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 31 Oct 2024 19:07:42 GMT, Chris Plummer wrote: >>> I do wonder if mutex support can be implemented for Windows with >>> Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >>> would be nice to have. Shame threads.h is not available with some Visual >>> Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >>> nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > >> > I do wonder if mutex support can be implemented for Windows with >> > Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it >> > would be nice to have. Shame threads.h is not available with some Visual >> > Studio versions we support, or at all with gcc. mtx_lock/unlock would be a >> > nice solution to this problem >> >> I'll also start looking into this in the meantime, unless you explicitly >> want me not to do so > > Feel free to if you'd like to. Personally I don't consider it to be that > important. @plummercj @alexeysemenyukoracle @sashamatveev reawaiting review - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2466039622
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 31 Oct 2024 19:07:42 GMT, Chris Plummer wrote: > I do wonder if mutex support can be implemented for Windows with > Acquire/ReleaseSRWLockExclusive. A `CriticalSection` is a mutex. A RWLock is not a "mutex". - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2451288744
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 31 Oct 2024 07:13:56 GMT, Julian Waters wrote: > > I do wonder if mutex support can be implemented for Windows with > > Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it > > would be nice to have. Shame threads.h is not available with some Visual > > Studio versions we support, or at all with gcc. mtx_lock/unlock would be a > > nice solution to this problem > > I'll also start looking into this in the meantime, unless you explicitly want > me not to do so Feel free to if you'd like to. Personally I don't consider it to be that important. - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2450630093
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Sat, 26 Oct 2024 04:35:09 GMT, Julian Waters wrote: > I do wonder if mutex support can be implemented for Windows with > Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it > would be nice to have. Shame threads.h is not available with some Visual > Studio versions we support, or at all with gcc. mtx_lock/unlock would be a > nice solution to this problem I'll also start looking into this in the meantime, unless you explicitly want me not to do so - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2449178499
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 24 Oct 2024 14:22:28 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8342682 I do wonder if mutex support can be implemented for Windows with Acquire/ReleaseSRWLockExclusive. I know it's not strictly needed, but it would be nice to have. Shame threads.h is not available with some Visual Studio versions we support, or at all with gcc. mtx_lock/unlock would be a nice solution to this problem - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2439331318
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 24 Oct 2024 14:22:28 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8342682 Turns out jpackage is part of core libs - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2439318255
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 24 Oct 2024 14:22:28 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8342682 I don't know what label jpackage falls under - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2437133404
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
On Thu, 24 Oct 2024 14:22:28 GMT, Julian Waters wrote: >> After 8339120, gcc began catching many different instances of unused code in >> the Windows specific codebase. Some of these seem to be bugs. I've taken the >> effort to mark out all the relevant globals and locals that trigger the >> unused warnings and addressed all of them by commenting out the code as >> appropriate. I am confident that in many cases this simplistic approach of >> commenting out code does not fix the underlying issue, and the warning >> actually found a bug that should be fixed. In these instances, I will be >> aiming to fix these bugs with help from reviewers, so I recommend anyone >> reviewing who knows more about the code than I do to see whether there is >> indeed a bug that needs fixing in a different way than what I did > > Julian Waters has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > 8342682 All the original labels are still there. A better approach had probably been to close the PR and open a new. You either need to do that now, or delete all the irrelevant labels. (I can't be bothered to figure out; at least build is wrong.) - PR Comment: https://git.openjdk.org/jdk/pull/21616#issuecomment-2436431780
Re: RFR: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage [v2]
> After 8339120, gcc began catching many different instances of unused code in > the Windows specific codebase. Some of these seem to be bugs. I've taken the > effort to mark out all the relevant globals and locals that trigger the > unused warnings and addressed all of them by commenting out the code as > appropriate. I am confident that in many cases this simplistic approach of > commenting out code does not fix the underlying issue, and the warning > actually found a bug that should be fixed. In these instances, I will be > aiming to fix these bugs with help from reviewers, so I recommend anyone > reviewing who knows more about the code than I do to see whether there is > indeed a bug that needs fixing in a different way than what I did Julian Waters has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8342682 - Changes: - all: https://git.openjdk.org/jdk/pull/21616/files - new: https://git.openjdk.org/jdk/pull/21616/files/e149e654..2294b27f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=21616&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=21616&range=00-01 Stats: 68 lines in 22 files changed: 1 ins; 21 del; 46 mod Patch: https://git.openjdk.org/jdk/pull/21616.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/21616/head:pull/21616 PR: https://git.openjdk.org/jdk/pull/21616
