Closed Bug 1093108 Opened 9 years ago Closed 9 years ago

Log shaking dumps erases logs if a folder with the same name exists

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 affected)

RESOLVED FIXED
2.2 S3 (9jan)
Tracking Status
b2g-v2.2 --- affected

People

(Reporter: jlorenzo, Assigned: gerard-majax)

References

Details

(Whiteboard: [systemsfe])

Attachments

(1 file, 3 obsolete files)

Pre-requisites
Have shake to log enabled

STR
1. Shake your device!
2. Retrieve your logs to a computer and note at what time they were save (YYYY-MM-DD-hh-mm-ss format)
3. Change the timezone to another and set the time to the one you had in 2.
4. Shake it up until your create logs with the same timestamp as in step 2.
5. Go to the logs folder

Expected result
You would have 2 folders to get the logs in step 1 and the ones in step 4.

Actual
The logs in step 2 are deleted. You can check for instance that dev-log-main.log doesn't have the same size in both cases (mine in step 4 were smaller than in step 1).

This is an edge case, that might be problematic edge case if you move through timezones.
As a similar case: screenshots don't have this issue. When you try to overwrite one (because of the same timestamp), you get the notification "Screenshot could not be saved NoModificationAllowedError".
I do see another usecase where it could be much more problematic: crossing timezones.

Maybe we should rather leave some time between two shaking? Does it makes sense that we do constant dumping if the users does constant shaking ?
Assignee: nobody → lissyx+mozillians
After offline discussion with johann, we came to the conclusion this is far from being a frequent edge case, so not working on this with high priority. If anyone comes with STRs that makes it more frequent, please raise your voice.
Whiteboard: [systemsfe]
The directory we are using to produce logs used by LogShake is computed
using local date/time. If for some reason the clock changes (traversing
timezones, daylight saving, bogus hardware, clock readjustment, ...) and
we are unlucky enough, then we may endup trying to write logs in a
directory that aready exists. We fix this by checking if there is
already a directory with this name, and if it turns out to be true, then
we append a digit to differentiate.
The directory we are using to produce logs used by LogShake is computed
using local date/time. If for some reason the clock changes (traversing
timezones, daylight saving, bogus hardware, clock readjustment, ...) and
we are unlucky enough, then we may endup trying to write logs in a
directory that aready exists. We fix this by checking if there is
already a directory with this name, and if it turns out to be true, then
we append a digit to differentiate.
Attachment #8537171 - Attachment is obsolete: true
Damn, OS.File.exists() returns a promise .... Everything is screwed.
Attachment #8537215 - Attachment is obsolete: true
So I tried just making use of OS.File.makeDir() with "ignoreExisting: false". Yet, this is not working and the directory is overwritten.

Yoric, do you see anything wrong in b2g/components/LogShake.jsm, saveLogs() function ? Below is the matching logcat output, where I forced the directory to be "timestamp" instead of the real timestamp value.

12-16 19:09:08.541  6162  6162 I GeckoDump: LogShake: handling event capture-logs-start
12-16 19:09:08.541  6162  6162 I GeckoDump: LogShake: handling capture-logs-start
12-16 19:09:09.071  6162  6162 I Gecko   : LogShake.jsm: making a directory all the way from /storage/sdcard to /storage/sdcard/logs/timestamp
12-16 19:09:09.191  6490  6490 I Gecko   : ###################################### forms.js loaded
12-16 19:09:09.261  6490  6490 I Gecko   : ############################### browserElementPanning.js loaded
12-16 19:09:09.311  6490  6490 I Gecko   : ######################## BrowserElementChildPreload.js loaded
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of properties
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/main
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/system
12-16 19:09:09.591  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/radio
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/events
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/cmdline
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/kmsg
12-16 19:09:09.601  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/meminfo
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/uptime
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/version
12-16 19:09:09.611  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmallocinfo
12-16 19:09:09.621  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmstat
12-16 19:09:09.841  6162  6162 I Gecko   : LogShake.jsm: returning logfilenames: ["logs/timestamp/properties.log", "logs/timestamp/dev-log-main.log", "logs/timestamp/dev-log-system.log", "logs/timestamp/dev-log-radio.log", "logs/timestamp/dev-log-events.log", "logs/timestamp/proc-cmdline.log", "logs/timestamp/proc-kmsg.log", "logs/timestamp/proc-meminfo.log", "logs/timestamp/proc-uptime.log", "logs/timestamp/proc-version.log", "logs/timestamp/proc-vmallocinfo.log", "logs/timestamp/proc-vmstat.log"]
12-16 19:09:09.841  6162  6162 I GeckoDump: LogShake: handling event capture-logs-success
12-16 19:09:09.841  6162  6162 I GeckoDump: LogShake: handling capture-logs-success

...

12-16 19:09:11.901  6162  6162 I GeckoDump: LogShake: handling event capture-logs-start
12-16 19:09:11.901  6162  6162 I GeckoDump: LogShake: handling capture-logs-start
12-16 19:09:12.381  6162  6162 I Gecko   : LogShake.jsm: making a directory all the way from /storage/sdcard to /storage/sdcard/logs/timestamp
12-16 19:09:12.451  6162  6162 I Gecko   : LogShake.jsm: requesting save of properties
12-16 19:09:12.451  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/main
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/system
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/radio
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /dev/log/events
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/cmdline
12-16 19:09:12.461  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/kmsg
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/meminfo
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/uptime
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/version
12-16 19:09:12.471  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmallocinfo
12-16 19:09:12.481  6162  6162 I Gecko   : LogShake.jsm: requesting save of /proc/vmstat
12-16 19:09:12.781  6162  6162 I Gecko   : LogShake.jsm: returning logfilenames: ["logs/timestamp/properties.log", "logs/timestamp/dev-log-main.log", "logs/timestamp/dev-log-system.log", "logs/timestamp/dev-log-radio.log", "logs/timestamp/dev-log-events.log", "logs/timestamp/proc-cmdline.log", "logs/timestamp/proc-kmsg.log", "logs/timestamp/proc-meminfo.log", "logs/timestamp/proc-uptime.log", "logs/timestamp/proc-version.log", "logs/timestamp/proc-vmallocinfo.log", "logs/timestamp/proc-vmstat.log"]
12-16 19:09:12.781  6162  6162 I GeckoDump: LogShake: handling event capture-logs-success
12-16 19:09:12.781  6162  6162 I GeckoDump: LogShake: handling capture-logs-success
Flags: needinfo?(dteller)
Simply rely on OS.File.makeDir() and gracefully fail instead of
overwriting pre-existing user data.
As discussed off-bugzilla, the issue is actually in the documentation of option `from` of `OS.File.makeDir`. Clearing needinfo.
Flags: needinfo?(dteller)
Simply rely on OS.File.makeDir() and gracefully fail instead of
overwriting pre-existing user data.
Attachment #8537311 - Attachment is obsolete: true
Attachment #8537852 - Flags: review?(anygregor)
Blocks: 1019816
Attachment #8537852 - Flags: review?(anygregor) → review+
Keywords: checkin-needed
this needs a try run i guess ? Could you provide one? Thanks!
Flags: needinfo?(lissyx+mozillians)
Keywords: checkin-needed
(In reply to Carsten Book [:Tomcat] from comment #12)
> this needs a try run i guess ? Could you provide one? Thanks!

Right, the one in comment 5 is not for the latest patch.

https://tbpl.mozilla.org/?tree=Try&rev=7911e19b51a7
Flags: needinfo?(lissyx+mozillians)
Keywords: checkin-needed
(In reply to Alexandre LISSY :gerard-majax from comment #13)
> (In reply to Carsten Book [:Tomcat] from comment #12)
> > this needs a try run i guess ? Could you provide one? Thanks!
> 
> Right, the one in comment 5 is not for the latest patch.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=7911e19b51a7

And it's green :)
https://hg.mozilla.org/mozilla-central/rev/9592f4f944bc
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
You need to log in before you can comment on or make changes to this bug.