Open Bug 1024525 Opened 5 years ago Updated 5 years ago

Add OS.File.appendAtomic() API


(Toolkit :: OS.File, defect)

Not set




(Reporter: Irving, Unassigned, Mentored)


(Blocks 1 open bug)


(Whiteboard: [lang=js][good next bug])


(1 file)

Log.jsm would like to be able to append entries to a log file efficiently and asynchronously, and not need to worry about managing the open/closed state of the file. Previous log-to-file code in AddonLogging.jsm did synchronous open/write/close on the main thread; because this was only used for ERROR-level messages it wasn't a significant source of jank, but we'd still like to make it async.
Marking this as a mentored bug.

Here are the steps:

1. in file osfile_shared_front.js, define a method `AbstractFile.appendAtomic` that opens a file/append some content/closes the file (see function `AbstractFile.writeAtomic`, already defined in the same file, as an example of opening, writing and closing);

2. in file osfile_async_worker.js, define a function `appendAtomic` that calls `File.appendAtomic` (see function `writeAtomic`, already defined in the same file, as an example);

3. in file osfile_async_front.jsm, define a function `appendAtomic` (see function `writeAtomic`, already defined in the same file, as an example).

All the source code resides here:
Mentor: dteller
Whiteboard: [lang=js][good next bug]

I am interested in working on this bug. So please can you assign this bug to me?

Thanks in advance,

Assignee: nobody → allamsetty.anup
Added the appendAtomic API to osfile as in the bug. 

Tested the code after adding it. Build succeeded with no erros.
Attachment #8457134 - Flags: review?(dteller)
Comment on attachment 8457134 [details] [diff] [review]
Added the appendAtomic API

Review of attachment 8457134 [details] [diff] [review]:

As mentioned over IRC, I'll only do a final review once we have tests.
The code looks good, although some of the comments copied and pasted from writeAtomic do not apply here.

::: toolkit/components/osfile/modules/osfile_async_front.jsm
@@ +1210,5 @@
>  };
> +/**
> + * Appends a file, atomically.
> + *

The following two paragraphs are not true.
Attachment #8457134 - Flags: review?(dteller) → feedback+
Hi Yoric,

I am sorry to say that I was not able to work on this bug for a while and I can continue working on this bug after a few days because now exams are going on for us. 

Sorry for the delay.

Flags: needinfo?(dteller)
Why did you needinfo me?
Flags: needinfo?(dteller)
Any news, Anup?
Flags: needinfo?(allamsetty.anup)
Hi Yoric,

I am sorry to say that it might delay a few days because I'm facing some personal issues. 

I can  complete this bug by writing the testcase for that within a week or two. 

Once again I'm sorry for the delay. 

Flags: needinfo?(allamsetty.anup)
Assignee: allamsetty.anup → nobody
You need to log in before you can comment on or make changes to this bug.