Closed Bug 175867 Opened 19 years ago Closed 19 years ago

CFM and mach-o profile locking mechanisms are different

Categories

(Core Graveyard :: Embedding: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.3alpha

People

(Reporter: saari, Assigned: ccarlen)

References

Details

Attachments

(1 file, 1 obsolete file)

CFM and mach-o profiile locking mechanisms are different, thus they cannot
coexist with the same profile
*** Bug 175869 has been marked as a duplicate of this bug. ***
Blocks: 176301
working on it
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2final
Attached patch patch (obsolete) — Splinter Review
Patch imports all the needed stuff from the BSD world in order to do the same
(fcntl) profile locking as Mach-0 build. I added GetCFURL() to nsILocalFileMac.
I needed it here and there's at least one other place I know of where I need
it. I'll make a separate bug for that.
Depends on: 179932
Attached patch patch v2Splinter Review
Patch makes both use fcntl locking. Improvement over last patch is that, after
succeeding with an fcntl lock, goes on to check the lock style that CFM has
used since mozilla 1.0. We don't have to worry about the other order of
launching because Mac mozilla and NS has used the Draconian check for other
running processes.
Attachment #104944 - Attachment is obsolete: true
-> 1.3
Target Milestone: mozilla1.2final → mozilla1.3alpha
Comment on attachment 106095 [details] [diff] [review]
patch v2

Bryner/Simon - r or sr. sorry - the only people familiar with this stuff are
srs.
Attachment #106095 - Flags: superreview?(sfraser)
Attachment #106095 - Flags: review?(bryner)
Comment on attachment 106095 [details] [diff] [review]
patch v2

>+static CFBundleRef getBundle(CFStringRef frameworkPath)
>+{
>+	CFBundleRef bundle = NULL;
>+    
>+	//	Make a CFURLRef from the CFString representation of the bundle's path.

Please don't use hard tabs.

>+    if (bsd_open && bsd_close && bsd_fcntl && bsd_error) // FALSE for TARGET_CARBON on OS9
>+    {
>+        // We need a UTF-8 Posix path to pass to open.
>+        CFURLRef lockFileCFURL;
>+        nsCOMPtr<nsILocalFileMac> lockMacFile(do_QueryInterface(lockFile));

Does nsIFile::GetNativePath not give you the right encoding?
The code is starting to look like an ugly nest of #ifdefs. Can the
platform-specific code be moved into subclasses of some virtual base class, to
make it cleaner?
Summary: CFM and mach-o profiile locking mechanisms are different → CFM and mach-o profile locking mechanisms are different
Bryner:
> Please don't use hard tabs.
  Tabs somehow creep in like cockroaches :-/ I'll get rid of them.
> Does nsIFile::GetNativePath not give you the right encoding?
  No - and it's an HFS, colon-delimited path too, which won't work here.

Simon:
  Yes, it's an ugly nest of #ifdefs. I think it should be moved out of this file
and into a static lib built by profile mgr but linkable to other code as well.
With profile sharing, we may need this code outside of this scope. I was hoping
to wait until that point to really clean this up, but...

Bryner, if this was in a static lib and that lib existed in several places, the
signal handlers used on Unix would be OK, right? Aren't the handlers called in
order and there can be any number of them for a given signal?
Comment on attachment 106095 [details] [diff] [review]
patch v2

Yes, they're chained.  When you install a signal handler, you get back the
previous handler that was installed for that signal.  It's then your
responsibility to call that handler from your handler.

r=bryner for this patch with the tabs removed.
Attachment #106095 - Flags: review?(bryner) → review+
Attachment #106095 - Flags: superreview?(sfraser) → superreview+
Fixed.

Simon, I made a version with a virtual base class and then platform-specific
derived classes. That required a concrete wrapper class whose destructor would
release the lock. It ended up being a more expensive, complex impl, and still a
lot of #ifdefs if the derived class impls are in 1 file. I'm not sure if I like
it - I'll keep it around, though. 
Status: ASSIGNED → RESOLVED
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.