Clojure

Remove birth-thread check from transients

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.6
  • Fix Version/s: Release 1.7
  • Component/s: None
  • Patch:
    Code
  • Approval:
    Ok

Description

Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:

user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread  clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)

Proposal: Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:

user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]

The clj-1498-3.diff version of the patch also replaces the AtomicReference<Thread> with AtomicBoolean as we can now track just ownership, not who owns it.

Doc update: Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients

Patch: clj-1498-3.diff

Alternative: Another idea would be to make this check optional with some kind of option on the transient call (transient coll :check-owner true). Not sure whether what the default would be for that.

  1. clj-1498.diff
    08/Aug/14 11:55 AM
    2 kB
    Alex Miller
  2. clj-1498-2.diff
    13/Aug/14 1:42 PM
    16 kB
    Alex Miller
  3. clj-1498-3.diff
    20/Aug/14 8:59 AM
    20 kB
    Alex Miller

Activity

Alex Miller made changes -
Field Original Value New Value
Assignee Alex Miller [ alexmiller ]
Fix Version/s Release 1.7 [ 10250 ]
Reporter Alex Miller [ alexmiller ] Rich Hickey [ richhickey ]
Approval Vetted [ 10003 ]
Alex Miller made changes -
Description Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.)

Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}
Alex Miller made changes -
Attachment clj-1498.diff [ 13212 ]
Alex Miller made changes -
Patch Code [ 10001 ]
Alex Miller made changes -
Description Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}
Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}

*Doc update:* Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients
Alex Miller made changes -
Attachment clj-1498-2.diff [ 13220 ]
Alex Miller made changes -
Description Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}

*Doc update:* Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients
Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}

The clj-1498-2.diff version of the patch also replaces the AtomicReference<Thread> with AtomicBoolean as we can now track just ownership, not who owns it.

*Doc update:* Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients

*Patch:* clj-1498-2.diff

*Alternative:* Another idea would be to make this check optional with some kind of option on the transient call {{(transient coll :check-owner true)}}. Not sure whether what the default would be for that.
Stuart Halloway made changes -
Approval Vetted [ 10003 ] Incomplete [ 10006 ]
Alex Miller made changes -
Attachment clj-1498-3.diff [ 13239 ]
Description Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}

The clj-1498-2.diff version of the patch also replaces the AtomicReference<Thread> with AtomicBoolean as we can now track just ownership, not who owns it.

*Doc update:* Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients

*Patch:* clj-1498-2.diff

*Alternative:* Another idea would be to make this check optional with some kind of option on the transient call {{(transient coll :check-owner true)}}. Not sure whether what the default would be for that.
Transients protect themselves from use by any thread other than the one that creates them. This is good for safety, however it eliminates certain valid usages of transients. For example, usage in a go-block might occur in subsequent invocations across multiple OS threads (but only one logical thread of control).

Current simple test:
{code}
user> (def v (transient []))
#'user/v
user> (persistent! @(future (conj! v 1)))
IllegalAccessError Transient used by non-owner thread clojure.lang.PersistentVector$TransientVector.ensureEditable (PersistentVector.java:464)
{code}

*Proposal:* Remove the owner check from transient collections. (Leave the edit after persistent check as is.) The test above should succeed.

After:
{code}
user=> (def v (transient []))
#'user/v
user=> (persistent! @(future (conj! v 1)))
[1]
{code}

The clj-1498-3.diff version of the patch also replaces the AtomicReference<Thread> with AtomicBoolean as we can now track just ownership, not who owns it.

*Doc update:* Various pieces of documentation will need to be updated with this change, namely http://clojure.org/transients

*Patch:* clj-1498-3.diff

*Alternative:* Another idea would be to make this check optional with some kind of option on the transient call {{(transient coll :check-owner true)}}. Not sure whether what the default would be for that.
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Stuart Halloway made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Status Open [ 1 ] Closed [ 6 ]
Resolution Completed [ 1 ]

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: