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

Threading issue with RteProject.refresh() #53

Open
cy-nsf opened this issue Dec 18, 2017 · 2 comments
Open

Threading issue with RteProject.refresh() #53

cy-nsf opened this issue Dec 18, 2017 · 2 comments

Comments

@cy-nsf
Copy link

cy-nsf commented Dec 18, 2017

Summary:

The CMSIS Pack Eclipse plugins for Eclipse as of 2.3.0 are not thread-safe. Specifically, we are having issues with project refresh.

Background:

All class names, file names, and line numbers are based on the 2.3.0 release of the CMSIS Pack Eclipse plugins.

As part of our project creation flow, we call RteProject.refresh() to make sure the CMSIS RTE has a change to update itself and our Eclipse project.

The implementation of this method winds up creating an Eclipse Job called RteProjectUpdater (at RteProjectManager.java:378). This job is derived from the Eclipse WorkspaceJob class, which is designed for tasks that need to atomically update an Eclipse workspace — so far so good. This job also explicitly configures itself to use the IWorkspace rule, which is also good.

It just so happens that the RteProjectManager class listens to the Eclipse resourceChanged “event”. The implementation in the CMSIS plugin winds up calling “.refresh” on the rteProject reference (RteProjectManager.java:328). This event is raised because of previous RteProjectUpdater job is running.

This winds up triggering a second RteProjectUpdater job, which is scheduled and run by Eclipse.

Problem:

Our tool has it’s own WorkspaceJob that runs as part of our project creation process. It renames the users project template selected by the user to use their desired project name.The failure mode we observe is:

  1. We call RteProject.refresh() to make selected / enabled CMSIS components from our project templates are visible and known to the CMSIS RTE.

  2. We attempt to rename the project to comply with the users given project name.

  3. The job from Unreported NPE in CMSIS-PACK plugin #1 creates the second RteProjectUpdater job.

We have a race condition. If the jobs run 1 3 2, everything works out just fine. CMSIS RTE is up to date, and the project is renamed, and life goes on. However, we frequently see the order 1 2 3. The rename in between #1 and #3 causes #3 to fail with the following error:

Title: Multiple problems have occurred.
Message: Error updating RTE project
Detail: "RTE Project Updater"

We need a way to make sure our project rename job isn’t scheduled by Eclipse until after all the CMSIS RteProject refresh jobs have been retired. The Eclipse WorkspaceJob / IWorkspace rule is not robust enough. There is a window between the two CMSIS refresh jobs where ours can get scheduled.

Thx.

@edriouk
Copy link
Contributor

edriouk commented Dec 19, 2017

Thank you for the detailed problem description. Obviously we have never tested such scenario.
Could you suggest a fix for this issue? Pull request is very velcomed.
~ Evgueni

@cy-nsf
Copy link
Author

cy-nsf commented Dec 19, 2017

Reindhard mentioned you needed instructions for reproducing the issue. It's not something that can be easily triggered in vanilla Eclipse. Essentially what you need to do are:

  1. Perform an action that triggers IRteProject.refresh().
  2. Immediately try and rename the project.

Unfortunately humans are too slow, and the two CMSIS plugin Eclipse jobs are retired before you can click through the Eclipse menus. We're triggering it because the IRteProject refresh / and IProject rename operations happen in code, back to back.

The easiest way to reproduce this is:

A. Create a new WorkspaceJob whose implementation uses the Eclipse RenameRefactoring object to rename a project.

B. Call IRteProject.refresh();

C. Create and schedule the job from A.

You'll notice that the call in B will create+schedule an RteProjectUpdater job. That job will eventually trigger the creation+scheduling of a second RteProjectUpdater job.

Sometimes Eclipse will retire the two RteProjectUpdater jobs back to back and everything appears to work just fine. Sometimes Eclipse will schedule the job from A above, and the second RteProjectUpdater will fail as described in the defect report.

The key problem is that the work you're doing to make the Eclipse workspace up to date gets split between two Eclipse Jobs. The Job rule that prevents multiple access to the Eclipse workspace is released when the first job finishes, and is re-acquired by the second job. This gives any other pending jobs the chance to sneak in and make a change (like rename the project) out from underneath the second Job.

Option 1: Make sure that all work required to refresh an IRteProject is completed as part of a single Job. You'll own access to the Eclipse workspace, 100% finish your work, and release it when done.

Option 2: Expose an Eclipse JobGroup. All CMSIS Job objects should add themselves to this job group. This allows outside code to call theJobGroupObject.join() -- which allows us to reliably wait until all jobs in the job group are retired before we schedule the project rename.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants