Null Pointer Exception When Saving a Model.

[emammoser]

Issue and tracking information

I don't have a special reproducer for this issue, but basically, when making some number of changes to a model and then saving it, we randomly get a null pointer exception and a message that says "Save Failed", even though the save does appear to have succeeded. Our users get very worried that their data isn't being saved. We would like to guarantee that the file is saved and not have this exception occur.

Here is an image of the stack trace:

Developer's time Estimated effort to fix (hours):

Developer's Actual time spent on fix (hours)

Issue reporter to provide a detailed description of the issue in the space below

[eposse]

This one looks related to Issue #390. Did you get this error with the most recent version or was it with an earlier, before we merged Pull Request #403?

[emammoser]

I got this error with papyrus built with the newest hash on the papyrus branch: https://github.com/ZeligsoftDev/CX4CBDDS/commit/77e92e9f61dec7181ab17cea2bb1638906a1dde5

[eposse]

Ok, so this is new. I'll investigate. It could be that like other NPEs it may be harmless, and if such, we can suppress it with the mechanism introduced by Pull Request #403. But I'll check that saving is working correctly first.

[eposse]

One more question, which one should have higher priority, this issue or #415?

[emammoser]

This issue as it hopefully has a faster turnaround. But if it is taking you a while to diagnose we can reconsider.

[eposse]

Ok.

[eposse]

I haven't been able to reproduce this one yet, but the Papyrus code strongly suggests that this exception occurs after the model has been saved. But there may be other consequences that are not entirely clear, as the code that doesn't get executed tells the command stack that the save is done, and informs listeners that the dirty status of the file changed.

It's not clear why the exception occurs, but I suspect some race condition deep inside plain Papyrus' code.

[ysroh]

I suspected that this issue might be related to resources that are loaded in multiple editors. I wasn't able to reproduce the exact same exception but I do see other exceptions when trying to work on a model that has an editable reference model and the referenced model is also open in another editor. These are known issues as the Papyrus does not handle these cases very well and that's why editing was disabled by default. I think we should disable the editing capability of the referenced model for end users or at least put a preference field to explicitly enable this for ICM developers.

[emammoser]

@ysroh Didn't we just recently enable the ability to edit the referenced model via. a dependent model? I believe this is something that our users want to be able to do. Unless of course I am thinking of something else.

[emammoser]

@ysroh is probably right with his suspicion. The model where I saw this issue was a model that references other ICMs in our workspace.

[ysroh]

Correct, We enabled this recently and know that this will cause exceptions if the referenced model is also open and you work on both models. If this capability is a feature needed for all end users then we just need to educate them well about the limitation so they do not open referenced model if the model is already loaded in the current model as a reference. but if this feature is really needed for only some users then I am suggesting turning this off by default since this feature is added for convenience and users are able to do everything without this feature by modifying the original model.

[emammoser]

I don't believe I had any other model open when I saw this issue, only the top level model. I know other people have seen this issue before which is why I wrote the ticket without a specific reproducer. I would say almost every user will want this capability so having it be an option isn't the best choice here.

If I am wrong, and this issue can only be reproduced when opening multiple models where the saving model references another, then maybe we can add a new warning dialog on the open of a model.

[ysroh]

I see. If you only had one model open then this issue is not related to the editing capability. It might be just another exception that we might want to filter out. I am very worried that users will see exceptions without knowing that they had multiple models open. It's not just for the ICM models since you can reference another model if the other model is located in the same project and users will likely work on both models in the same project. I am trying to think how we can keep the number of exceptions down and hopefully filter out all unwanted ones.

[ghost]

The ability to edit a referenced model was explicitly added by issue #368 back in April 2022 as a desired and standard workflow improvement. Lots of discussion and rationale there. https://github.com/ZeligsoftDev/CX4CBDDS/issues/368

[ysroh]

Understood. We will try to filter out all unwanted exceptions that do not impact the model since I really don't like to see the exceptions. @eposse Can you filter out this exception without reproducing it?

[ysroh]

And I know that most exceptions do occur during the reloading stage after saving is done.

[eposse]

Ok, I'll add the exception to the filter.

[eposse]

I've opened PR #427.

[emammoser]

@eposse Can you give any status on this issue?

[eposse]

Hi. I don't have any updates on this. I can't reproduce it, and from the discussion above, @ysroh suggested that it might be related to multiple models open, they refer to each other and they are edited, but later it is mentioned that the exception occurs when only one model is opened. @ysroh said that we would filter out that exception, which is what I did with PR #427. This PR filters out the exception shown in your original screenshot. If there are other exceptions that need to be filtered, let us know and I'll add them. But without being able to reproduce it, I'm not sure what else we could do at this point.

[emammoser]

Can we merge in the PR?

[eposse]

Yes, I'll do that.

[eposse]

It's merged and the build can be obtained here.

Shall we close this one and reopen if we get a more reliable reproducer?

[emammoser]

Yes, I think that sounds like a good plan