Page MenuHomePhabricator

[dashboard] mitk::NavigationToolStorageDeserializer uses directory without write permission on windows continous dart client
Closed, ResolvedPublic

Description

this leads to random fails. Maybe the directory from deprecated mitk::StandardFileLocations is not safe any more. Changing to mitk::IOUtil::CreateTemporaryDirectory() might help.

Event Timeline

User franza has pushed new remote branch:

bug-17303-NavigationToolStorageDeserializer

User franza has pushed new remote branch:

bug-17303-NavigationToolStorageDeserializerV2

[c4652b]: Merge branch 'bug-17303-NavigationToolStorageDeserializerV2'

Merged commits:

2014-02-21 11:03:32 Alfred Franz [f6f063]
COMP(#17303): disabled tests because of T17181 and T17303


2014-02-21 11:00:29 Alfred Franz [2354a4]
Merge branch 'bug-17303-NavigationToolStorageDeserializer' into bug-17303-NavigationToolStorageDeserializerV2


2014-02-21 10:56:14 Alfred Franz [0e5bb4]
COMP(#17303): Started conversion of deserialzer to IOUtil, not working yet

franza added a comment.Mar 5 2014, 3:31 PM

[d6c691]: Merge branch 'bug-17303-NavigationToolStorageDeserializer'

Merged commits:

2014-02-21 18:37:51 Alfred Franz [3268af]
now using the mitk program path to permanently store the toolfiles. Maybe this helps to avoid errors.


2014-02-21 18:21:45 Alfred Franz [a52373]
replaced call of CreateTemporaryDirectory() by call with parameters, so it is not affected by T17310


2014-02-21 17:21:23 Alfred Franz [e2bddc]
replaced dirty hack by poco implementation

franza added a comment.Mar 5 2014, 3:32 PM

merged all the fixes.

still TODO: activate the test again!

wegner added a subscriber: wegner.Jul 18 2016, 4:39 PM

By the way, not only the continuoud dart client has the problem but also the user of an installed application!

I just found out that a crash a user reported is caused by the same bug. The NavigationToolStorage tries to serialize the storage and crashes during serialization. The user can proceed after confirming an error box telling to manually save the data and shut down the system. During the next startup the application crashes when starting the plugin.

The NavigationToolStorage is supposed to be stored in the cache directory, right?

I won't open up another bug as this it would be basically the same as this bug.

Cheers ;)
Ingmar

kislinsk removed franza as the assignee of this task.Aug 3 2016, 8:09 PM
kislinsk triaged this task as Unbreak Now! priority.
kislinsk added subscribers: franza, kirchnth.
kislinsk edited subscribers, added: clarkson, seitela, kislinsk; removed: git-user.Aug 3 2016, 8:12 PM

This bug was also upvoted by @clarkson on our mailing list today. I raised the priority to Unbreak Now!.

Hi Alfred.

I think its this line of code:

https://github.com/MITK/MITK/blob/master/Modules/IGT/IO/mitkNavigationToolStorageDeserializer.cpp#L39

We create our app, create an installer using nsis, and install it on another machine. The default installation dir is therefore under C:\Program Files, which is only writable by the Administrator.

So, when a non-priviledged user runs the app, this deserialisation code tries to create a temporary file under C:\Program Files rather than under C:\Temp.

So it should be

mitk::IOUtil::GetTempPath()

instead of

mitk::IOUtil::GetProgramPath()

Thanks

Matt

franza added a comment.Aug 5 2016, 4:32 PM

Hi Matt,

thanks for this hint. It should be easy to change but is possibly not the reason for the original bug which occured randomly on our dart clients.

Regards,

Alfred

franza claimed this task.Aug 5 2016, 4:41 PM

changed the temp directory location to mitk::IOUtil::GetTempPath() as proposed by Matt and activated the test again. Lets see if this also fixes the random crashes of the tests.

Did it work?

franza added a comment.Aug 9 2016, 1:32 PM

I don't know yet. We are still working on the phabricator migration and the continuous clients are not fully functional at the moment. On my PC the test works fine. But the problem was a random fail that appeared on the continuous client once a week or less, so we have to wait to see if it happens again.

Did you check if the fix solves your problem?

Steve Thompson, here at UCL, just tested it, and reported that it does appear to fix our problem. Thanks.

kislinsk lowered the priority of this task from Unbreak Now! to High.Aug 9 2016, 5:23 PM

Any news on this?

kislinsk closed this task as Resolved.Sep 1 2016, 10:09 AM

As far as I see, the test hasn't failed since this task was merged. However, there wasn't much activity on our dashboards anyway. I close this task for now. Please reopen, if necessary.