#1500 BugSplat Crash when out of disk space #2004
Conversation
LLGuru
force-pushed
the
guru/viewer-1500-crash-when-out-of-disk-space
branch
from
01c0298
to
9ff8f98
Compare
LLGuru
force-pushed
the
guru/viewer-1500-crash-when-out-of-disk-space
branch
from
9ff8f98
to
36a6f2f
Compare
indra/llcommon/llapr.cpp
Outdated
| @@ -574,6 +575,9 @@ S32 LLAPRFile::readEx(const std::string& filename, void *buf, S32 offset, S32 nb | |||
| S32 LLAPRFile::writeEx(const std::string& filename, const void *buf, S32 offset, S32 nbytes, LLVolatileAPRPool* pool) | |||
| { | |||
| LL_PROFILE_ZONE_SCOPED; | |||
| if (LLApp::isFileReadOnlyMode()) | |||
There was a problem hiding this comment.
Operations will just fail with no reason yet viewer will keep allocating threads for writing?
Also this isn't the only way of writing to cache, textures use Apr because it's supposed to be thread friendly.
If you want to make viewer read only, it's better to switch cache to read only. Futhermore if you are offering to switch viewer to read only mode, it should specify to user what that wuld do and how to turn it back on.
There was a problem hiding this comment.
What do you mean writing "read only"? The field LLAppViewer::mSecondInstance ?
mSecondInstance is not used exclusively to prevent writing to the disk,
it seems to be used in absolutely different algorithms
And writing to the disk could happen in many cases without testing this flag
Also I'm not sure that we may turn it on in the middle of the viewer session
Viewer with mSecondInstance uses other directories for writing files
That's why I decided that we'd better introduce a new separate flag,
semantically corresponding to the general purpose: prevent storing data to the disk
And yes, there are many points where viewer writes files,
so the current commit is just a declaration of the idea, but not a final it's implementation
It is still a draft
indra/llcommon/llerror.cpp
Outdated
| LLUserWarningMsg::Handler LLUserWarningMsg::sHandler; | ||
| // LLOutOfMemoryWarning, LLOutOfDiskWarning | ||
| // need to be preallocated before viewer runs out of memory/disk | ||
| static std::string sLocalizedOutOfMemoryTitle; |
There was a problem hiding this comment.
sLocalizedOutOfMemoryTitle and showOutOfMemory() were preallocated (arguably should have been inline as well) because it's out of memory handling, to not allocate once we run out. There is no point prerecording these strings for everything else.
indra/llcommon/llerror.cpp
Outdated
| @@ -1623,6 +1628,21 @@ namespace LLError | |||
| } | |||
| } | |||
|
|
|||
| void LLUserWarningMsg::showOutOfDisk() | |||
| { | |||
There was a problem hiding this comment.
If you call this from main thread it will put thread on pause and will prevent viewer from sending 'keep alive' packets. Running out of disk space is a non critical issue, it should not abruptly interrupt user's experience, instead it should be a popup in the corner that recommend to free some space.
Calling it from a thread will span user with a cascade of messages since there are multiple threads and all can have the same issue.
LLGuru
force-pushed
the
guru/viewer-1500-crash-when-out-of-disk-space
branch
from
36a6f2f
to
e98f94d
Compare
indra/llcommon/llapp.cpp
Outdated
| if (now - recently_sent < min_interval) | ||
| return; | ||
|
|
||
| recently_sent = now; |
There was a problem hiding this comment.
Small suggestion:
static U32Seconds next_notification = U32Seconds(0);
static U32Seconds min_interval = U32Seconds(60);
U32Seconds now = LLTimer::getTotalTime();
if (now < next_notification)
return;
next_notification = now + min_interval;
b0f32e0
to
e253b7c
Compare
LLGuru
force-pushed
the
guru/viewer-1500-crash-when-out-of-disk-space
branch
from
e253b7c
to
39e54ef
Compare
LLGuru
force-pushed
the
guru/viewer-1500-crash-when-out-of-disk-space
branch
from
39e54ef
to
21f965f
Compare
In case of out-of-disk-space error the viewer will display the following notification:
(not more often than once per 60 seconds)