Use the main AbstractFuture implementation under GWT? #2934

cpovirk · 2017-09-01T19:42:42Z

I haven't thought about how practical this would be, but it ought to be doable with enough supersourcing.

It would mean shipping more code to GWT, though, since the server copy of AbstractFuture is larger. (It would also mean that we can't make some of the methods final in the GWT version, but making them final was just something I did opportunistically, not a requirement.)

The upside: We might get features and bug fixes. For example, I just heard that some GWT code has a problem with stack overflow because it didn't have the server copy's protections in complete() (compare GWT's notifyAndClearListeners()). We've also failed to update the GWT copy when updating the server copy (e.g., when fixing a bug in pendingToString()). Ideally we'd always catch these problems with tests, but occasionally we don't have them, or maybe we have them but not for GWT because they're convenient to write with threads.

The text was updated successfully, but these errors were encountered:

cpovirk · 2019-09-03T19:20:17Z

I think there might be a bug here:

guava/guava-gwt/src-super/com/google/common/util/concurrent/super/com/google/common/util/concurrent/AbstractFuture.java

Lines 409 to 417 in 551207b

    
           try { 
        
             forceSet(getDone(delegate)); 
        
           } catch (ExecutionException exception) { 
        
             forceSetException(exception.getCause()); 
        
           } catch (CancellationException cancellation) { 
        
             cancel(false); 
        
           } catch (Throwable t) { 
        
             forceSetException(t); 
        
           }

An Error thrown not just by getDone but by forceSet (by a listener, specifically) would be caught, and the value of the future would then be changing from the original successful output to the failure. (I think -- I haven't tested.)

…er ExecutionException Also fix GWT AbstractFuture so that it respects the trusted interface. (More motivation for #2934) RELNOTES=N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=282674203

…ackage -- make them consistent with the underlying classes. This gives us slightly more motivation for #2934. RELNOTES=`util.concurrent`: Improved nullability annotations for J2cl overlays. PiperOrigin-RevId: 492223286

…ackage -- make them consistent with the underlying classes. This gives us slightly more motivation for #2934. RELNOTES=n/a PiperOrigin-RevId: 492223286

…ackage -- make them consistent with the underlying classes. This gives us slightly more motivation for #2934. RELNOTES=n/a PiperOrigin-RevId: 493966031

…g it for more environments. That includes J2KT and [GWT/J2CL](#2934). Changes include: - Suppress nullness at smaller scopes, fixing errors that were hidden by the old, broad suppression. - Remove `synchronized` from an override of `fillInStackTrace`. J2KT doesn't like `synchronized` except on very specific types, and we don't need it. - Introduce `rethrowIfErrorOtherThanStackOverflow` and `interruptCurrentThread`, `Platform` methods that will require different implementations for J2KT and J2CL/GWT. - Introduce helper methods like `casListeners(expect, update)` for `AtomicHelper` operations. These reduce verbosity relative to `ATOMIC_HELPER.casListeners(this, expect, update)`. They also prepare for `AbstractFuture` implementations that don't use `AtomicHelper`. - Introduce `notInstanceOfSetFuture`. This is arguably nicer than `!(localValue instanceof SetFuture)`, but the real purpose is to prepare for when code in a different file needs to check `instance SetFuture`. (I could be convinced that I should just make `SetFuture` package-private instead, even though no one outside the file should use it for anything but an `instanceof` check.) - Mysteriously move things around, increase visibility of members, and introduce and sometimes use accessors. This is to prepare for when some of the code will be moving to a separate file so that the remainder of `AbstractFuture` can be wholly shared across different platforms. - Fix a few typos in comments. Also, rename `SetFuture`. This isn't directly related, but now is as good a time as any to do it. Additional bonus: This CL probably makes [the logging at the "end" of `AbstractFuture` static initialization](https://github.com/google/guava/blob/7ec362ec68b630363231d5292cd6b2577c710be6/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L210) have a better chance of actually working in the hypothetical situation that a logger uses `AbstractFuture`: Currently, `AbstractFuture` performs some further initialization _after_ that logging (such as the initialization of `NULL`). Now, it performs all that initialization before the `static` block that might log. RELNOTES=n/a PiperOrigin-RevId: 727068949

…om GWT/J2CL. This CL introduces a superclass, `AbstractFutureState`, following the pattern of [`AggregateFutureState`](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/AggregateFutureState.java). That superclass contains platform-specific operations. Fixes #2934 RELNOTES=n/a PiperOrigin-RevId: 726495822

…g it for more environments. That includes J2KT and [GWT/J2CL](#2934). Changes include: - Suppress nullness at smaller scopes, fixing errors that were hidden by the old, broad suppression. - Remove `synchronized` from an override of `fillInStackTrace`. J2KT doesn't like `synchronized` except on very specific types, and we don't need it. - Introduce `rethrowIfErrorOtherThanStackOverflow` and `interruptCurrentThread`, `Platform` methods that will require different implementations for J2KT and J2CL/GWT. - Introduce helper methods like `casListeners(expect, update)` for `AtomicHelper` operations. These reduce verbosity relative to `ATOMIC_HELPER.casListeners(this, expect, update)`. They also prepare for `AbstractFuture` implementations that don't use `AtomicHelper`. - Introduce `notInstanceOfSetFuture`. This is arguably nicer than `!(localValue instanceof SetFuture)`, but the real purpose is to prepare for when code in a different file needs to check `instance SetFuture`. (I could be convinced that I should just make `SetFuture` package-private instead, even though no one outside the file should use it for anything but an `instanceof` check.) - Mysteriously move things around, increase visibility of members, and introduce and sometimes use accessors. This is to prepare for when some of the code will be moving to a separate file so that the remainder of `AbstractFuture` can be wholly shared across different platforms. - Fix a few typos in comments. Also, rename `SetFuture`. This isn't directly related, but now is as good a time as any to do it. Additional bonus: This CL probably makes [the logging at the "end" of `AbstractFuture` static initialization](https://github.com/google/guava/blob/7ec362ec68b630363231d5292cd6b2577c710be6/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L210) have a better chance of actually working in the hypothetical situation that a logger uses `AbstractFuture`: Currently, `AbstractFuture` performs some further initialization _after_ that logging (such as the initialization of `NULL`). Now, it performs all that initialization before the `static` block that might log. RELNOTES=n/a PiperOrigin-RevId: 727068949

…om GWT/J2CL. This CL introduces a superclass, `AbstractFutureState`, following the pattern of [`AggregateFutureState`](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/AggregateFutureState.java). That superclass contains platform-specific operations. Fixes #2934 RELNOTES=n/a PiperOrigin-RevId: 726495822

…g it for more environments. That includes J2KT and [GWT/J2CL](#2934). Changes include: - Suppress nullness at smaller scopes, fixing errors that were hidden by the old, broad suppression. - Remove `synchronized` from an override of `fillInStackTrace`. J2KT doesn't like `synchronized` except on very specific types, and we don't need it. - Introduce `rethrowIfErrorOtherThanStackOverflow` and `interruptCurrentThread`, `Platform` methods that will require different implementations for J2KT and J2CL/GWT. - Introduce helper methods like `casListeners(expect, update)` for `AtomicHelper` operations. These reduce verbosity relative to `ATOMIC_HELPER.casListeners(this, expect, update)`. They also prepare for `AbstractFuture` implementations that don't use `AtomicHelper`. - Introduce `notInstanceOfSetFuture`. This is arguably nicer than `!(localValue instanceof SetFuture)`, but the real purpose is to prepare for when code in a different file needs to check `instance SetFuture`. (I could be convinced that I should just make `SetFuture` package-private instead, even though no one outside the file should use it for anything but an `instanceof` check.) - Mysteriously move things around, increase visibility of members, and introduce and sometimes use accessors. This is to prepare for when some of the code will be moving to a separate file so that the remainder of `AbstractFuture` can be wholly shared across different platforms. - Fix a few typos in comments. Also, rename `SetFuture`. This isn't directly related, but now is as good a time as any to do it. Additional bonus: This CL probably makes [the logging at the "end" of `AbstractFuture` static initialization](https://github.com/google/guava/blob/7ec362ec68b630363231d5292cd6b2577c710be6/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L210) have a better chance of actually working in the hypothetical situation that a logger uses `AbstractFuture`: Currently, `AbstractFuture` performs some further initialization _after_ that logging (such as the initialization of `NULL`). Now, it performs all that initialization before the `static` block that might log. RELNOTES=n/a PiperOrigin-RevId: 727068949

…g it for more environments. That includes J2KT and [GWT/J2CL](#2934). Changes include: - Suppress nullness at smaller scopes, fixing errors that were hidden by the old, broad suppression. - Remove `synchronized` from an override of `fillInStackTrace`. J2KT doesn't like `synchronized` except on very specific types, and we don't need it. - Introduce `rethrowIfErrorOtherThanStackOverflow` and `interruptCurrentThread`, `Platform` methods that will require different implementations for J2KT and J2CL/GWT. - Introduce helper methods like `casListeners(expect, update)` for `AtomicHelper` operations. These reduce verbosity relative to `ATOMIC_HELPER.casListeners(this, expect, update)`. They also prepare for `AbstractFuture` implementations that don't use `AtomicHelper`. - Introduce `notInstanceOfSetFuture`. This is arguably nicer than `!(localValue instanceof SetFuture)`, but the real purpose is to prepare for when code in a different file needs to check `instance SetFuture`. (I could be convinced that I should just make `SetFuture` package-private instead, even though no one outside the file should use it for anything but an `instanceof` check.) - Mysteriously move things around, increase visibility of members, and introduce and sometimes use accessors. This is to prepare for when some of the code will be moving to a separate file so that the remainder of `AbstractFuture` can be wholly shared across different platforms. - Fix a few typos in comments. Also, rename `SetFuture`. This isn't directly related, but now is as good a time as any to do it. Additional bonus: This CL probably makes [the logging at the "end" of `AbstractFuture` static initialization](https://github.com/google/guava/blob/7ec362ec68b630363231d5292cd6b2577c710be6/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L210) have a better chance of actually working in the hypothetical situation that a logger uses `AbstractFuture`: Currently, `AbstractFuture` performs some further initialization _after_ that logging (such as the initialization of `NULL`). Now, it performs all that initialization before the `static` block that might log. RELNOTES=n/a PiperOrigin-RevId: 729313044

…om GWT/J2CL. This CL introduces a superclass, `AbstractFutureState`, following the pattern of [`AggregateFutureState`](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/AggregateFutureState.java). That superclass contains platform-specific operations. Fixes #2934 RELNOTES=n/a PiperOrigin-RevId: 726495822

…). RELNOTES=n/a PiperOrigin-RevId: 729518831

…). RELNOTES=n/a PiperOrigin-RevId: 729532679

cpovirk added the package=concurrent label Sep 1, 2017

ronshapiro assigned cpovirk Sep 13, 2017

cpovirk removed their assignment Sep 13, 2017

cpovirk added the P3 no SLO label Sep 14, 2017

cgdecker added the platform=gwt label Sep 19, 2017

raghsriniv removed P3 no SLO labels Jun 24, 2019

ronshapiro added type=enhancement Make an existing feature better P3 no SLO labels Aug 8, 2019

cpovirk mentioned this issue Sep 3, 2019

AbstractFuture.afterCommit? #2886

Open

nick-someone mentioned this issue Nov 27, 2019

MOE Sync 2019-11-27 #3723

Merged

copybara-service bot mentioned this issue Dec 6, 2022

Improve nullability annotations for J2cl overlays in the concurrent package -- make them consistent with the underlying classes. #6262

Closed

copybara-service bot mentioned this issue Feb 20, 2025

Refactor the main AbstractFuture implementation to prepare for using it for more environments. #7691

Merged

copybara-service bot mentioned this issue Feb 20, 2025

Use most of the main AbstractFuture implementation from J2KT and from GWT/J2CL. #7692

Merged

copybara-service bot closed this as completed in b15c23f Feb 21, 2025

copybara-service bot pushed a commit that referenced this issue Feb 21, 2025

Assorted cleanups noticed after recent AbstractFuture [changes](#2934…

c904218

…). RELNOTES=n/a PiperOrigin-RevId: 729518831

copybara-service bot mentioned this issue Feb 21, 2025

Assorted cleanups noticed after recent AbstractFuture [changes](https://github.com/google/guava/issues/2934). #7697

Merged

copybara-service bot pushed a commit that referenced this issue Feb 21, 2025

Assorted cleanups noticed after recent AbstractFuture [changes](#2934…

8f6f4b3

…). RELNOTES=n/a PiperOrigin-RevId: 729532679

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the main AbstractFuture implementation under GWT? #2934

Use the main AbstractFuture implementation under GWT? #2934

cpovirk commented Sep 1, 2017

cpovirk commented Sep 3, 2019

Use the main AbstractFuture implementation under GWT? #2934

Use the main AbstractFuture implementation under GWT? #2934

Comments

cpovirk commented Sep 1, 2017

cpovirk commented Sep 3, 2019