Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added MAX_TRIES to SetContents of NbClipboard as well #8024

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wumpz
Copy link
Contributor

@wumpz wumpz commented Dec 5, 2024

Since in some previous commit the retry logic for getContents was rewritten, I was wondering, why this was not done with setContent. This PR does this.

However, the old version startet a setContents with a delay of 100 ms. So a setContents could possibly interfere, even another setContents, since a new task was created. Using this new retry logic this interference cannot happen.

@eirikbakke
Copy link
Contributor

Thanks for the suggested patch! Are you on a Windows machine yourself?

If you are trying to solve #7051, I'd recommend the following:

  1. Test the patch for a few weeks to see if the problem goes away as a result or not.
  2. If the problem did indeed go away as a result of the PR, now run NetBeans with the patch off again, and verify that the problem comes back.
  3. Re-introduce the patch and run it again for a while, and re-verify that the problem is gone again.

This seems to be the only way to troubleshoot this particular bug.

@wumpz
Copy link
Contributor Author

wumpz commented Dec 6, 2024

@eirikbakke Yes, I am using Windows and indeed I try to solve or improve on #7051. So how can I follow your recommended steps. Is it enough to simply replace the I think bootstrap.nbm in my Netbeans installation or do I have to do more?

@eirikbakke
Copy link
Contributor

Here's what I usually do when I want to have multiple custom NetBeans builds available to test and switch between:

  1. Clone the NetBeans repo.
    git clone [email protected]:apache/netbeans.git
  2. Switch to the branch of the latest official release. E.g. "git checkout 23"
  3. Run "ant". (Takes a few minutes.)
  4. Now there is a fresh NetBeans build in the directory netbeans\nbbuild\netbeans. Copy it e.g. to "C:\Program Files\NetBeans\netbeans-23"
  5. Now apply the changes you want to test.
    E.g. "wget https://patch-diff.githubusercontent.com/raw/apache/netbeans/pull/8024.patch" and "patch -p1 < 8024.patch" for this PR if you have a Linux command line available (via WSL).
  6. Run "ant clean" and "ant" again.
  7. Now you have another fresh NetBeans build in the directory netbeans\nbbuild\netbeans. Copy it e.g. to "C:\Program Files\NetBeans\netbeans-23-pr8024"
  8. Now you can run either "C:\Program Files\NetBeans\netbeans-23\bin\netbeans64.exe" or "C:\Program Files\NetBeans\netbeans-23-pr8024\bin\netbeans64.exe" to run with your patch disabled or enabled. (You may need to set netbeans_jdkhome to the path to your JDK in etc\netbeans.conf in each NetBeans folder.)

@wumpz
Copy link
Contributor Author

wumpz commented Dec 8, 2024

So I am now on a patched version of Apache Netbeans 23. I used this little program and got to 50 tries multiple times without a glitch: #3962 (comment). I had this problem before.
I will report back after some time. However, since the GetContent max tries patch was accepted, this one should as well, since it removes a structural problem.

if (i == (MAX_TRIES - 1) || (System.currentTimeMillis() + delay - start) > 1000L) {
throw ex;
} else {
log.log(Level.INFO, "systemClipboard#getContents ISE, attempt {0}", i + 1); // NOI18N
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log message should say setContents instead of getContents here.

@eirikbakke
Copy link
Contributor

Thanks for testing! Seeing whether the change makes a difference or not over time is very useful here.

I used this little program and got to 50 tries multiple times without a glitch: #3962 (comment). I had this problem before.

That's a good thing to test, in addition to regular IDE usage over some time. If you can, please try to switch to the NetBeans version that does not have the patch, and see if the test then starts producing the bug. I remember trying that particular test myself at some point without exhibiting the bug.

Another good thing to check for is to see that and IllegalStateException is actually thrown for setContents every now and then. This is to make sure the patch actually has some effect. To see this you'd need to change the log message in the patch that currently reads "systemClipboard#getContents ISE, attempt {0}" to "systemClipboard#setContents ISE, attempt {0}", to distinguish it from the getContents case.

setContents is a mutating operation unlike getContents, so I'd be a little more cautious about doing unconditional retries, unless we know that it fixes the bug. Probably we'd want to decrease the retry count a bit; though for testing purposes it can be useful to leave it as it is in the current version of the patch.

@eirikbakke eirikbakke added the Platform [ci] enable platform tests (platform/*) label Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants