On May 5, 10:44 am, Scott Hess <[email protected]> wrote:
> I agree with the earlier argument about not larding startup with
> things like writing new files to id the coming-up Chrome to
> late-coming instances. An alternative might be to acquire a lock to
> protect the profile, and write an id asynchronously after startup.
> The late-coming instance would see the lock and wait for the id to
> appear (or the lock to evaporate). This does mean the late-coming
> instance will have to wait sometimes, but in those cases the coming-up
> instance presumably is not ready to respond to requests anyhow. On
> Mac and Linux, all we need for the lock is an agreed-upon open file
> descriptor, which should be easy to find. We could acquire the lock
> in parallel with other startup activity which doesn't modify system
> state.
>
> -scott
>
>
>
> On Tue, May 5, 2009 at 10:16 AM, Amanda Walker <[email protected]> wrote:
>
> > Thanks for elaborating.
>
> > So if we look at it another way, there are several different problems
> > that ProcessSingleton is currently used to solve:
>
> > - Prevent multiple instances of Chromium from referring to the same
> > profile/user data directory. In the general sense, what we need here
> > is a way to acquire and test locks on data directories. This is a
> > cross-platform requirement: all platforms should be able to launch
> > instances of Chromium in different user sessions with different user
> > data / profiles (in order to support fast user switching on Mac &
> > Windows, single Linux boxes with multiple people logged in, etc.
> > Currently, ProcessSingleton really only solves this by accident. It
> > seems cleaner to me to meet this requirement with an explicit
> > mechanism: Something like a "ProfileLock" or "DirectoryLock," for
> > example. This also gives a way to ease this requirement if someone
> > fixes the concurrent-access problem, without affecting other uses of
> > ProcessSingleton.
>
> > - Testing, since a number of tests are not multiple-instance safe. I
> > would describe this as a defect in those tests or the test framework,
> > which again ProcessSingleton only mitigates by accident. I would
> > prefer we address this by fixing the tests and/or test scripts so that
> > we can indeed run tests in parallel without worrying about them
> > stepping on each other. We have a strong desire to reduce our build
> > cycle time, of which having to run tests serially instead of in
> > parallel is a large component.
>
> > - Within a user session, ensuring that requests to open URLs get
> > processed by a currently running instance of Chromium instead of
> > launching a new one. This is where platform differences are
> > strongest, and is ProcessSingleton's main function. Registering
> > properties on windows or the like for Windows and X11 seems cleaner to
> > me than putting breadcrumbs into the file system, since window
> > properties are inherently tied to the user session and to an active
> > instance of the application (and thus do not need to be cleaned up on
> > a crash, etc.). On the Mac, this function is provided by the OS, so
> > there's no need for Chromium to duplicate it.
>
> > --Amanda
>
> > On Tue, May 5, 2009 at 12:27 PM, Никита Офицеров <[email protected]> wrote:
>
> >> I'm sorry for the late response, I've been quite busy last week.
>
> >> 2009/4/27 Amanda Walker <[email protected]>:
> >>> "Hacky" is fairly subjective: are there particular things about the
> >>> existing implementation that bother you? Currently they are different
> >>> for each OS because each OS has its own issues surrounding launching
> >>> multiple instances of the browser, different mechanisms for handling
> >>> incoming requests from the OS to open web pages, and so on.
>
> >> 2009/4/28 stoyan <[email protected]>:
> >>> Well, on Windows code looks racy, does not work well across multiple
> >>> desktops, and probably will create troubles accessing userdata when
> >>> same instance is running in different terminal sessions (with same
> >>> user account).
>
> >> The problem with quick, baroque source code is that usually there are
> >> lots of corner cases.
> >> I have rethought the idea after your comments, and there are several
> >> problems with current
> >> Windows implementation:
>
> >> 1. As Stoyan mentioned, window messaging does not work across multiple
> >> desktops.
> >> Therefore it doesn't work across multiple terminal sessions.
>
> >> Consider the following use case: A user has a version of Windows that
> >> allows multiple
> >> consequent terminal sessions on his PC, and leaves Chrome opened. Then
> >> he opens a
> >> session from his laptop and launches Chrome. Preferred result is
> >> opening a new window
> >> in the second session, but AFAIK it's impossible for the Chrome master
> >> process to open a
> >> window in a different window station.
I know of no browser that would create tabs/windows across sessions
backed by the same process. Come to think of it I know of no product
that does this. Maybe some service that interacts with the user.
Hence quite a lot of changes are
> >> required for this to
> >> function, like delegating handling of a widget to the newly created
> >> process, and these fall
> >> out of the scope of this idea.
But current ProcessSingleton will just
> >> fail to detect another
> >> Chrome instance and crash or show weird behavior. If it worked, we
> >> could at least show some
> >> message telling user that running Chrome from different terminal
> >> sessions is not yet implemented.
Chrome actually does show a message. This is because the second
instance fails to open the relevant files (with no sharing) because of
the older instance has them.
> >> So IMHO ProcessSingleton should not rely on window messages, that are
> >> by definition
> >> bound to one window station.
>
> >> 2. Another problem with Windows implementation (maybe not really a
> >> problem) is that it
> >> guards Chrome against being launched with the same (luckily,
> >> case-insensitive) user data
> >> dir path. This is a fundamental problem, because ProcessSingleton
> >> should protect not only
> >> against being launched with the same path, but against actually using
> >> the same dir twice
> >> (one directory can be addressed with different paths, because of links
> >> and mounting, even on
> >> Windows NTFS partitions).
> >> Well, I think that without user intervention a situation that exploits
> >> this cannot be created, so
> >> it may be considered as ok, but it's still a problem that IMHO should be
> >> fixed.
Please fill a bug. I would think that is low priority.
>
> >> Because of that, I suggest two ideas:
> >> 1. Do almost as I suggested before, but instead of Chrome PID use a
> >> GUID, and write it only once,
> >> on the first run. This GUID will let us distinguish data dirs without
> >> need to rely on their path. So at
> >> normal launch it will be one additional disk read (just several bytes)
> >> and some IPC. If you think that
> >> we can't afford even one extra disk read, then think about my second idea:
>
> >> 2. If problem #2 is not really a problem, then instead of GUID or PID
> >> we can just use some hash from
> >> user data dir path as channel_id, and make no disk operations at all.
> >> This is by no means worse than
> >> current implementation and solves the first problem. However, this is
> >> only a partial solution.
>
> >> As an additional benefit, ProcessSingleton becomes what it was meant
> >> to be - a facility to ensure that
> >> only one instance with given data dir is running. Opening new URLs and
> >> getting browser PID will be
> >> just IPC messages, without custom protocol parsing or some extra
> >> quirks in ProcessSingleton code.
The singleton has its issues but it does a decent job for most people.
You are proposing a radical re-write to fix some corner cases. Maybe
they are important to you or to somebody. Certainly using our IPC will
make things cleaner. I would still file bugs and see what priority
they get.
>
> >> 2009/4/27 Amanda Walker <[email protected]>:
> >>> Simply eliminating differences between per-platform implementations is
> >>> not necessarily a large benefit by itself, since the amount of code is
> >>> small and concerns an area where the three platforms behave quite
> >>> differently.
> >> I do agree with this, but it's not only for eliminating differences
> >> between implementations,
> >> but also to improve them. There is already quite a lot of
> >> platform-specific code in IPC,
> >> and it can handle differences quite well. The only remaining thing is
> >> disabling this functionality
> >> completely on Mac when not running tests, and this can be handled by one
> >> 'if'.
>
> >> As for "racyness", this can also be improved. Current implementation
> >> was racy because singleton
> >> was created with some delay after checking that no Chrome process
> >> existed. That was a bug, and
> >> that was fixed by adding singleton creation in constructor. It isn't
> >> actually atomic, but should work
> >> well. Creating named pipe is atomic operation now
> >> (FILE_FLAG_FIRST_PIPE_INSTANCE),
> >> however, creating UNIX domain socket is not. So this may also be
> >> improved in order to avoid
> >> race conditions.
>
> >> Nikita Ofitserov
--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: [email protected]
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
