New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758634 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 882906
issue 754861



Sign in to add a comment

fcntl F_SETLK et al. return ENOSYS on Fuchsia

Project Member Reported by scottmg@chromium.org, Aug 24 2017

Issue description

Advisory file locking isn't implemented on Fuchsia in fcntl.

This causes e.g. failures in AppCacheDatabaseTest.* in content_unittests.

https://fuchsia.atlassian.net/browse/MG-264 (similar request for LevelDb) is WontFix, under the assumption that it's not necessary because access is only from one process so it's just to notify unrelated apps they shouldn't be mucking around.

Is it the case that we don't really need that locking in sqlite for Chromium, under the assumption that we'd only be accessing from a single process (browser) and a single thread (maybe?).

So could we stub the lock out entirely? Or maybe it an in-process thing if it does need to guard against multiple thread access but not multi-process access?

+pwnall, +michaeln for owners of sql/ and third_party/sqlite.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21b917d0199706826e1e8cb48a89535b78ebcfaf

commit 21b917d0199706826e1e8cb48a89535b78ebcfaf
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Aug 24 19:42:15 2017

fuchsia: triage some content_unittests failures

TBR=wez@chromium.org

Bug:  758634 ,  754861 
Change-Id: I82f6cc27d6fae34d998580dea92646dac4f5d865
Reviewed-on: https://chromium-review.googlesource.com/633889
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497157}
[modify] https://crrev.com/21b917d0199706826e1e8cb48a89535b78ebcfaf/testing/buildbot/filters/fuchsia.content_unittests.filter

Comment 2 by pwnall@chromium.org, Aug 24 2017

You'll need locking for SQLite. WebSQL uses SQLite directly from the renderer process, and it's possible to have multiple renderers for the same origin, which would potentially access the same database.

SQLite has some pretty good documentation on this topic at https://sqlite.org/lockingv3.html

I think that you can get away with an in-process locking implementation, similarly to what Fuchsia did for SQLite, if you can afford to turn WebSQL off.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4309df854fd178fa2c7da30130d4047d88e3b0e4

commit 4309df854fd178fa2c7da30130d4047d88e3b0e4
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Sep 07 21:26:06 2017

fuchsia: Disable DomStorageAreaParamTest in content_unittests

NOTRY=true

TBR: sergeyu@chromium.org
Bug:  754861 ,  758634 
Change-Id: Ibd48b5ec82559ba2ea589886d3e666ce04f4c710
Reviewed-on: https://chromium-review.googlesource.com/656065
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500386}
[modify] https://crrev.com/4309df854fd178fa2c7da30130d4047d88e3b0e4/testing/buildbot/filters/fuchsia.content_unittests.filter

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7ca6493e4de60565aaabb221783c57e6a88f99e

commit a7ca6493e4de60565aaabb221783c57e6a88f99e
Author: Scott Graham <scottmg@chromium.org>
Date: Tue Sep 12 18:51:18 2017

fuchsia: Use dot-file locking for sqlite

There's no advisory file locking, cross-process mutex or semaphore, or
implemented flock on Fuchsia currently. So fallback to using dot-file
locking for the time being.

Bug:  758634 
Change-Id: I7be88b0930eca3d4527e6ff161914987ac2150cb
Reviewed-on: https://chromium-review.googlesource.com/661321
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501346}
[modify] https://crrev.com/a7ca6493e4de60565aaabb221783c57e6a88f99e/third_party/sqlite/amalgamation/sqlite3.c
[add] https://crrev.com/a7ca6493e4de60565aaabb221783c57e6a88f99e/third_party/sqlite/patches/0011-fuchsia-Use-dot-file-locking-for-sqlite.patch
[modify] https://crrev.com/a7ca6493e4de60565aaabb221783c57e6a88f99e/third_party/sqlite/src/src/os_unix.c

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59f7de672b717c9f226c348b762bc8784f581219

commit 59f7de672b717c9f226c348b762bc8784f581219
Author: Scott Graham <scottmg@chromium.org>
Date: Fri Sep 15 21:05:23 2017

fuchsia: Enable file-lock dependent sqlite tests

Bug:  758634 
Change-Id: If5e43b56ad6d08272436ea9c47a57a011a4a8efb
Reviewed-on: https://chromium-review.googlesource.com/661318
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502368}
[modify] https://crrev.com/59f7de672b717c9f226c348b762bc8784f581219/testing/buildbot/filters/fuchsia.content_unittests.filter

Blocking: 882906
Status: Fixed (was: Started)
This has mostly been worked around. Crashpad still could use it, but work on the Fuchsia-stalled out, and that's separate from the Chrome problems which have been addressed sufficiently for now.

Sign in to add a comment