Closed Bug 185706 Opened 21 years ago Closed 19 years ago

Create a transaction manager module for the IPC Daemon to allow sharing of profile data


(Core Graveyard :: Profile: BackEnd, defect)

Not set


(Not tracked)



(Reporter: mozilla, Assigned: mozilla)



(Whiteboard: in IPC_BRANCH_20030304)


(2 files, 4 obsolete files)

bug 178806 discusses an IPC daemon for communication between two or more running
mozilla processes. A module needs to be written for the IPC daemon to convey
changes in profile information. 

The transaction manager needs a client side piece that can be talked to through
XPCOM by services that represent profile data ( like the pref service or cookie
service ) as well as a server side module for the IPC daemon that receives the
transactions and sends the data to any processes interested in the changes.

Documentation on the profile sharing problem and this solution can be found at:
,however the tech spec documents are slightly out of date already. (I'll be
updating them soon - this week)
This tarball contains a transactionmngr directory. It needs the code from
darin's ipc daemon to be built (see bug ) in modules/ipc. This code expects to
be placed in modules alongside the ipc directory (modules/transactionmngr). The
placement is just temporary until we find a final resting place for ipc and
it's modules.

This contains a test app: tmModuleTest, that spins up the ipc daemon and a
client that sends messages through the tmTransactionService (client side)
across the ipc layer and to the tmTransactionModule (in the ipcDaemon). There
are some different modes you can run it in: debug, listener, broadcaster ...
that are documented in the file.
whoops forget to add the bug number in the last comment, it is of coures bug
178806 the dependency for this bug.
Attached file ready for review (obsolete) —
Cleaned up patch, ready for review. 

untars to ./transactionmngr/

like the previous, goes in modules like the ipc code of darins (see dependency)

oh, haven't tested this on linux or mac, but will be checking that out this
afternoon, tomorrow AM at the latest.
Attachment #111843 - Attachment is obsolete: true
Comment on attachment 112827 [details]
ready for review

don't stick things in modules.
don't include Makefile.
don't include *.dll *.exe *.exp *.ilk *.obj *.lib *.pdb *.res _xpidlgen
do include a .cvsignore which would prevent those files from appearing
don't license files NPL
don't copyright files 1998
don't use PR_ASSERT (use NS_ASSERTION &| NS_ENSURE_...)
tmTransaction::SetRawMessage(const PRUint8 *aRawMessage, PRUint32 aLength) {
  mData = new tmMsg();
  mData->action = *index;
don't crash when new fails!
NS_INIT_ISUPPORTS(); is obsolete, don't use it.

javadoc style is:
 * like this
  * like this

I don't think tmUtils.h should include prlog.h/stdio.h, but that's a personal

tmIPCModule::Init() {
  if (tm == nsnull)
    tm = new tmTransactionManager();
what happens if new fails here?
ah, here we go:
tmIPCModule::HandleMsg(ipcClientHandle client, const nsID &target, const void
*data, PRUint32 dataLen) {
  tmTransaction *trans = new tmTransaction(IPC_GetClientID(client),
					   (PRUint8 *)data,
!! you don't check to see if new fails
  if (tm == nsnull)
!! you crash if init fails

tmQueue::PostTransaction(tmTransaction *aTrans) {
  mTM->SendTransaction(ownerID, new tmTransaction(ownerID,

I hope sendtransaction is null safe for its second param.
of course it isn't:
void tmIPCModule::SendMsg(PRUint32 aDestClientIPCID, tmTransaction
*aTransaction) {
	      (void *)aTransaction->GetRawMessage(),
!!crash here

tmVector::tmVector() : mIterator(0), mIndex(0), mNext(0) , mSize(0),
		       mGrowthIncrement(3) {
  mElements = new void*[mCapacity];
you shouldn't allocate memory in constructors, it may fail.
PRUint32 tmVector::Add(void *aElement){
  mElements[mNext] = aElement;
!! crash here.

Methods in idl should be interCaps not InitialCaps:

interface tmITransactionService : nsISupports
void Init(in ACString aProfileName);

  qm->queueName = PL_strdup(PromiseFlatCString(aQueueName).get());
if you were willing to trade PL_strfree for nsMemory::Free you could use
ToNewCString(aQueueName) and save an alloc.

tmModule.h claims to be part of IPC:
 * The Original Code is Mozilla IPC.
i suppose it is, but is this really what it should say?

why are you including #if 0 code in tmModule.cpp?

template <class Type>
mPtr<Type>& mPtr<Type>::operator=(const mPtr<Type>& mp) {
  if (this != &mp) {
    delete t;
    t = new Type(*(mp.t));  // assumes the assignment operator is good for the
  return *this;

This code is fragile.  Most people assume operator= will succeed. but if new
Type(..) fails, then the assignment will fail.


  // helps with shutdown on some cases
this is broken. but i suppose i can fix it later.
Attachment #112827 - Attachment is obsolete: true
Attachment #112827 - Attachment is patch: false
Attachment #112827 - Attachment mime type: text/plain → application/x-gzip
Attachment #112827 - Flags: review-
Attached file gzip of revised code (obsolete) —
addressed timeless's comments, conrad's point about the component thing,
plugged some leaks, cleaned a lot of crap up and doc'd almost everything. have
specific replies in a file I'm going to attach next
Attached patch patch to make TM build (obsolete) — Splinter Review
This is a patch to and that adds the neccessary foo
to make the transaction manager code build. You must define MOZ_TM=1 in your
environment and have unpacked the tarball so that it looks like:
Oops - I should have mentioned that I had done something like this. It goes a
little further though: It adds a autoconf switch which makes a #define, and
turns on MOZ_IPC as well.
Comment on attachment 115160 [details] [diff] [review]
patch to make TM build

This patch isn't quite right and doesn't apply because I had to edit it.

Instead, you can pull the PROFILE_SHARING_1_BRANCH and build there! :-)
Attachment #115160 - Attachment is obsolete: true
This is a patch (really) containing the revisions of Darin's review of the
transaction manager IPC module. The code is also in it's new home, the
ipc/ipcd/extensions/ directory.
Attachment #114987 - Attachment is obsolete: true
Comment on attachment 120409 [details] [diff] [review]
patch of transmngr ipc module

Looks good. The doc in the header files is really nice. Just a few nits...

>? Makefile
>? build/Makefile
>? common/Makefile
>? module/Makefile
>? public/Makefile
>? src/Makefile
>? test/Makefile

Don't forget to check in a .cvsignore file for each directory containing a That will prevent the '?' output. Of course, if using Windows,
you're gonna be buried by the other '?' output.

>Index: build/tmModule.cpp
>+#if 0
>+tmTransactionServiceRegisterProc(nsIComponentManager *aCompMgr,
>+                       nsIFile *aPath,
>+                       const char *registryLocation,
>+                       const char *componentType,
>+                       const nsModuleComponentInfo *info)

Might as well get rid of this #if 0 code. I doubt we will ever need to load the
component in this way. Even if we did, I think the preferred way is to use the
stuff in embedding/components/appstartup.

>Index: common/tmVector.cpp
>Index: common/tmVector.h

A nice, compact implementation, but why couldn't nsVoidArray be used?

>Index: public/tmITransactionService.idl
>+  void attach(in ACString aDomainName, 
>+              in tmITransactionObserver aObserver, 
>+              in PRBool aLockingCall);

Something should be said about what sort of reference the transaction service
keeps to the observer. It's just a plain weak reference, which people might not

>Index: src/tmTransactionService.cpp
>+tm_waiting_msg::~tm_waiting_msg() {
>+  printf("destroying a tm_waiting_msg struct\n");

Put that in #if DEBUG. There are a few other printfs in this file and we
wouldn't want to drag in StdCLib just for that.

>+tmTransactionService::~tmTransactionService() {
>+  // just destroy this, it contains 2 pointers it doesn't own.
>+  if (mObservers)
>+    PL_HashTableDestroy(mObservers);
>+  PRUint32 index = 0;
>+  PRUint32 size = mWaitingMessages.Size();
>+  tm_waiting_msg *msg = nsnull;
>+  for ( ; index < size; index ++) {
>+    msg = (tm_waiting_msg*) mWaitingMessages[index];
>+    if (msg)

No need for the "if" with delete.

>+      delete msg;
>+  }
Attachment #120409 - Flags: review+
Comment on attachment 120409 [details] [diff] [review]
patch of transmngr ipc module

super review from darin?
Attachment #120409 - Flags: superreview?(darin)
so i just have one big nit about the transaction service.  specifically, since
it has been moved into the IPC module, i think the IDL prefix should be switched
from "tm" to "ipc".  not necessary to change it throughout, but i think renaming
the interfaces would be a good thing. likewise, the contract id should be 


or something like that.  also, i think you want to do away with building
transaction service as its own XPCOM DLL.  it should just be statically built
into the ipcdc XPCOM DLL (just like what is done with the lock service extension).
Whiteboard: in IPC_BRANCH_20030304
Comment on attachment 120409 [details] [diff] [review]
patch of transmngr ipc module

sr=darin fwiw... all this code is on the IPC branch anyways.  i just need to
flip it over to the trunk at some point.  but, this bug is essentially fixed i
Attachment #120409 - Flags: superreview?(darin) → superreview+
This has been on the trunk for a while now... marking FIXED.
Closed: 19 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.